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.

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

Implement occlusion culling #2

Open
Rendez opened this issue Sep 11, 2017 · 14 comments
Open

Implement occlusion culling #2

Rendez opened this issue Sep 11, 2017 · 14 comments
Assignees
Milestone

Comments

@Rendez
Copy link
Contributor

Rendez commented Sep 11, 2017

The initial idea was and continues to be the existence to two render types:

  • incremental (current implementation)
  • infinite (occlusion culling)

If anyone would like to give the implementation a try, please go ahead

@Rendez Rendez added this to the 1.0.0 milestone Sep 11, 2017
@Rendez Rendez self-assigned this Sep 11, 2017
@Rendez Rendez modified the milestones: 1.0.0, 2.0.0, 3.0.0 Nov 28, 2017
@setsun
Copy link

setsun commented Feb 1, 2018

Virtualization would be great. I'll see if I can give this a shot if I have time.

An implementation thought I have so far:

  • When IntersectionObserver is triggered at bottom of list, take the current scrollTop and load in a div with that height and swap that with the divs from the previous page.
    • Would require a separate sentinel (IntersectionObserver) at the top of the list if user scrolls up.
    • Not sure how much scroll jank this implementation could have.

Would appreciate any other ideas people have.

@Rendez
Copy link
Contributor Author

Rendez commented Feb 5, 2018

Great, I'm happy to assist you on this!

I gave the implementation some thought a while ago, up until a certain point. However, since then I hadn't had the chance to come back to it... So, here's were my thoughts on it:

  • It would use two sentinels: one at the beginning and another one at the end of the list.
  • If a sentinel is suddently intersecting, then the top & height sizes would be extracted using the callback argument's entry. These two readonly props should work: IntersectionObserverEntry.boundingClientRect and IntersectionObserverEntry.rootBounds.

The current List class should not need much overhauling, and its current structure would be similar.

This approach assumes that initially all of the rendered items' height is immutable and will not change.

I think it's worth starting off in a sandbox environment, where we can throw in our ideas together. If you prefer to go ahead on your own, that's also fine.

@setsun
Copy link

setsun commented Feb 5, 2018

I didn't know that about IntersectionObserver that is actually pretty convenient. We are exploring using this in production, and it would be great to use this over react-virtualized for large sets of data.

I wonder if this should come after the v2.0.0 changes, or be back-ported? I notice that the API may change via this PR: #19

@Rendez
Copy link
Contributor Author

Rendez commented Feb 6, 2018

I'm glad to hear that. We actually started this project in order to stop using react-virtualized, which is overkill for most common use cases. v2.0.0 is important because it'll be the most stable release yet.

The v2.0.0 branch won't change, and is practically ready. I haven't merged it yet since we wanted to test it live in production, but it's due very soon, so branch off from there and I'll make sure it gets merged this week.

@setsun
Copy link

setsun commented Feb 7, 2018

Great, I'll branch off of that PR and I'll try to get a work-in-progress PR sometime within the work week that we can discuss and work off of. Thanks for the clarification!

@setsun
Copy link

setsun commented Feb 12, 2018

@Rendez just an update, currently only had a little time to work on this, but I have a branch on my fork listed here (very much WIP): https://github.com/Setsun/react-intersection-list/tree/feature/virtualization

@MatteoGioioso
Copy link

Hey, sorry to come late to the party, I was just playing around with the implementation a little bit.
https://codesandbox.io/s/virtualized-list-2l0eu
This is really raw, I worked on it for one hour or so, take it for what is it 😄
I just noticed with my current implementation if I scroll too fast downwards the sentinel cannot catch up. I will see how to solve this, when I have time next week.

@Rendez
Copy link
Contributor Author

Rendez commented Jul 12, 2019

@MatteoGioioso nice try Matteo, the impl. would be something along the line of what you're presenting here. I don't see however items being removed from the list when I scroll up again, which would be the intent...
Scrolling too fast downwards will never give us a responsive enough solution because it will always depend on the main thread AND on the device power, so on slow mobile devices this delay could take several seconds, specially if the main thread is busy. This is due to the nature of the IntersectionObserver, which is lazy by default. For this reason the folks at Google gave up on using the IntersectionObserver for infinite lists, and went back to 'onscroll' event handlers (which combined with Houdini), provide a good result. The new <virtual-scroller> is supposed to solve this very complicated problem on the web platform.

@MatteoGioioso
Copy link

@Rendez

I don't see however items being removed from the list when I scroll up again

Yea I will try to implement it next time I will put my hands on it, did not have the time 😄

Scrolling too fast downwards will never give us a responsive enough solution because it will always depend on the main thread

So what would be a good solution? Do you have any suggestions?
I was thinking about adding another sentinel to swap, but it was just a thought.

@Rendez
Copy link
Contributor Author

Rendez commented Jul 12, 2019

So what would be a good solution? Do you have any suggestions?

There is no "good" solution because the IntersectionObserver impl. in browsers is too delayed, we must use onscroll. In a way, this library won't play that well with a responsive occlusion culling, though at the beginning I thought it would. I would recommend react-window now.

There is one way though in which the same could be accomplished with the IntersectionObserver API, which would be a combination of requestAnimationFrame() and takeRecords(). This way we would make it "blocking" and fast while waiting for a microtask to be scheduled thus not totally blocking user interaction. This check would need to be done on an endless loop OR attached to onscroll with throttle.

@MatteoGioioso
Copy link

MatteoGioioso commented Jul 16, 2019

@Rendez Ok thanks, I will look into it. I just like the concept of using IntersectionObserver, is much more simple than traditional approach such react-virtualized or react-window
However reading back I noticed that maybe there was a misunderstanding. When I said:

I just noticed with my current implementation if I scroll too fast downwards the sentinel cannot catch up

I should have said: if I scroll too fast downwards the sentinel fails completely to remove rows.
If you notice on my demo if you scroll downwards too fast the rows are not removed. But I think this might be an error in my implementation rather than IntersectionObserver, could you please confirm? 😄

@Rendez
Copy link
Contributor Author

Rendez commented Jul 16, 2019

I agree that the concept is great, I'm looking forward to what you'll come up with @MatteoGioioso!

@HenrikBechmann
Copy link

Hey @Rendez, check out https://github.com/HenrikBechmann/react-infinite-grid-scroller/blob/master/src/cradle.tsx starting around line 305 for how I use IntersectionObserver to cycle through visible content in a scroller.

Seems to work (modern browsers, reasonably fast hardware).

All comments welcome!

@lishine
Copy link

lishine commented Apr 12, 2020

Hey @Rendez, check out https://github.com/HenrikBechmann/react-infinite-grid-scroller/blob/master/src/cradle.tsx starting around line 305 for how I use IntersectionObserver to cycle through visible content in a scroller.

Seems to work (modern browsers, reasonably fast hardware).

All comments welcome!

He actually implemented a solution, maybe it will help you.
HenrikBechmann/react-infinite-grid-scroller#1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants