Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

SignalServiceKit refactor rendering glitches #1146

Closed
michaelkirk opened this issue Apr 11, 2016 · 4 comments
Closed

SignalServiceKit refactor rendering glitches #1146

michaelkirk opened this issue Apr 11, 2016 · 4 comments
Assignees
@michaelkirk
Milestone
2.3

Comments

@michaelkirk
Copy link
Contributor

michaelkirk commented Apr 11, 2016 •
edited

This issue is mostly a placeholder for rendering glitches blocking release as reported by @FredericJacobs

Eg. Messages from previous thread that you were viewing, appearing in the one you are currently viewing.

Some maybe relevant commits:
@michaelkirk michaelkirk self-assigned this Apr 11, 2016
@michaelkirk michaelkirk added this to the 2.3 milestone Apr 11, 2016
@michaelkirk
Copy link
Contributor Author

I don't really notice any glitches when viewing short or plain text threads, however as threads get longer and include more media, things get really slow, and frames start to drop.

Doing some analysis in instruments showed we're spending a lot of time building our TSInteractions. As a quick first step, I've cached them here:
https://github.com/WhisperSystems/Signal-iOS/compare/master...michaelkirk:fix-glitchy-ui?expand=1

@FredericJacobs can you see if this improves the situation that you were experiencing?

There seems to be some more low hanging optimization fruit that I will continue to attack.

@FredericJacobs
Copy link
Contributor

No, I'm not talking about dropped frames.

When you have a few threads and switch back and forth between them, some messages from other threads might be rendered in the wrong thread.

@michaelkirk
Copy link
Contributor Author

michaelkirk commented Apr 28, 2016 •
edited

I have never reproduced this, so I am going to let it float until the beta. Hopefully getting more users involved will get us more reproduction data.

michaelkirk added a commit that referenced this issue May 16, 2016
@michaelkirk
Fix composition box size when switching threads.
6252b4d 
Partial revert of 2c83046 which
introduced a shared reusable message view controller across threads.

2c83046 resulted in several discovered
bugs so far (#1179, #1150, #1152, and maybe: #1146). It's pretty clear
at this point we're going against the grain of how
JSQMesageViewController is intended to be used, and since the nominal
purpose of this feature (iPad Layout) doesn't exist, we should revert to
the known good way of interacting with the MessageViewController,
creating a fresh instance per thread.

// FREEBIE
michaelkirk added a commit that referenced this issue May 17, 2016
@michaelkirk
Reset compose view on thread switch (#1187)
9bacc3d 
* all Signal users can send text messages, never hide texting toolbar.

//FREEBIE

* Fix composition box size when switching threads.

Partial revert of 2c83046 which
introduced a shared reusable message view controller across threads.

2c83046 resulted in several discovered
bugs so far (#1179, #1150, #1152, and maybe: #1146). It's pretty clear
at this point we're going against the grain of how
JSQMesageViewController is intended to be used, and since the nominal
purpose of this feature (iPad Layout) doesn't exist, we should revert to
the known good way of interacting with the MessageViewController,
creating a fresh instance per thread.

// FREEBIE
@michaelkirk
Copy link
Contributor Author

AFAIK this was not reproduced. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@michaelkirk michaelkirk

Labels
None yet
Milestone
2.3
Development

No branches or pull requests
2 participants
@michaelkirk @FredericJacobs