Skip to content
This repository was archived by the owner on Feb 2, 2023. It is now read-only.

Working range window#371

Merged
secretiverhyme merged 1 commit intomasterfrom
working-window
Mar 12, 2015
Merged

Working range window#371
secretiverhyme merged 1 commit intomasterfrom
working-window

Conversation

@rnystrom
Copy link
Copy Markdown
Contributor

This brings back the concept of a window store for nodes that are in the working range (reverting #127). It turns out that due to the system architecture if there are nodes who fetch remote content (e.g. ASNetworkImageNode), calls to -display will occur before fetching has been completed. The next chance the nodes have to decode and display content is then when they are actually on the screen, thus defeating the purpose of a working range.

With the reintroduction of the working range window, nodes are "stored" in the window and when content is finished being fetched, CA triggers -display since they are part of a view hierarchy.

This can be tested in the Kittens project by insuring that before ASRangeController adds a node to a visible view that the image node (with remote content) has set its layer's contents.

@secretiverhyme
Copy link
Copy Markdown
Contributor

Let's move the

  [node recursivelySetDisplaySuspended:NO];

call out of the range controller and into ASRangeHandlerRender, then land this!

@rnystrom
Copy link
Copy Markdown
Contributor Author

Also making a note from @appleguy that thrashing a UIWindow like this will likely incur some wasted processing on the main thread. Something we should be aware of and fix soon.

@secretiverhyme
Copy link
Copy Markdown
Contributor

Yup — the hierarchy manipulation thrash is specifically why we wanted to kill the rendering-window hack. We're going to have to either convert the render range to "continuously re-display everything as needed" (matching the rendering window behaviour) or set up something like the state machine described in #315, so nodes can know whether they're supposed to render themselves once preload data arrives.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just to be absolutely safe, can we add an ASDisplayNodeAssertMainThread() here?

@secretiverhyme
Copy link
Copy Markdown
Contributor

Two tiny nits, then we're good to merge. =]

This brings back the concept of a window store for nodes that are in the working range (reverting #127). It turns out that due to the system architecture if there are nodes who fetch remote content (e.g. `ASNetworkImageNode`), calls to `-display` will occur before fetching has been completed. The next chance the nodes have to decode and display content is then when they are actually on the screen, thus defeating the purpose of a working range.

With the reintroduction of the working range window, nodes are "stored" in the window and when content is finished being fetched, CA triggers `-display` since they are part of a view hierarchy.

This can be tested in the Kittens project by insuring that before `ASRangeController` adds a node to [a visible view](https://github.com/facebook/AsyncDisplayKit/blob/master/AsyncDisplayKit/Details/ASRangeController.mm#L57) that the image node (with remote content) has set its layer's contents.
@rnystrom
Copy link
Copy Markdown
Contributor Author

Nits have been picked!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants