[Home] Eager clipping of rails
alloy opened this issue · 9 comments
@majak Hey, yep that’s correct. I haven’t dived into debugging it yet, but it sounds like you are already aware of that limitation? Got any tips for where to start looking?
Ideally we don't want to traverse complete uiview hierarchy to figure out which views should be clipped and which not. So we do a small optimization: If a view should be clipped since its outside of the clipping rect we remove it without looking at its subviews.
This is pretty reasonable, but doesn't work in a case where you would have a subview of the clipped view, which is outside of the clipped view's bounds and falls into the clipping rect (so it should be visible).
IIRC this matches what we do on Android and what we are currently doing (=before the patch you've applied).
Solution here would be to modify your view hierarchy in a way that you won't have this disappearing view outside of its parent bounds. Does it sound doable?
(If this sounds like unreasonable limitation we could think about adding a way to manually prevent clipping for "special" views.)
Ideally we don't want to traverse complete uiview hierarchy to figure out which views should be clipped and which not.
Aye, agreed.
So we do a small optimization: If a view should be clipped since its outside of the clipping rect we remove it without looking at its subviews.
[…]
IIRC this matches what we do on Android and what we are currently doing (=before the patch you've applied).
Oh indeed, I just went back to check this and I now see it, I hadn’t realised this before. There’s a 20pt difference when the view is removed after applying the patch, though, so that made it more apparent, but it’s still removed too early indeed. [Before | After]
Solution here would be to modify your view hierarchy in a way that you won't have this disappearing view outside of its parent bounds. Does it sound doable?
Fair point, will take a look at that.
If this sounds like unreasonable limitation we could think about adding a way to manually prevent clipping for "special" views.
I hear what you’re saying, but I wouldn’t be a big fan of having to somehow declare that special case. I’m wondering if most cases couldn’t be solved by:
- having about 1 page worth of content ready, i.e. not remove subviews until they are more than 1 time the content size away?
- or if the total actual bounding box of all subviews couldn’t be calculated/cached when the shadow views calculate their layouts?
@majak Ok, so I indeed solved our issue by reworking the hierarchy a bit to ensure the button lies within its superview’s bounds 👍
Nonetheless, if needed, I’d like to volunteer some of my time to help you perfect this change. It’s not entirely clear to me if you’re at FB and are internally working at it there? In which case I understand the ongoing work is not done in public.
Hey @alloy, it's great to hear you could rework your view hierarchy!
I'm at FB :) Current state of this issue is that I'm finishing a recursive implementation.
It would be really helpful if you could try that out once I publish a new patch on the issue (should be this week). The main concern for the recursive clipping is perf. It shouldn't be bad (initial testing on our lists shows roughly 3ms of clipping work per frame instead of current 2ms on iPhone5), but it's inherently more work.
@majak Awesome! I’d love to test it out, let me know when it’s available and I’ll do so immediately.
Hey @alloy, the new thing is in master now. If you are not running on master apply these three commits locally:
facebook/react-native@4783930
facebook/react-native@625c8cb
facebook/react-native@d5e067f