Davejkane / riv

Riv - The Rust Image Viewer
MIT License
99 stars 10 forks source link

Glob with progress #95

Open gurgalex opened 5 years ago

gurgalex commented 5 years ago

This PR aims to resolve #89

Getting progress indications from a long-running glob isn't the most mentally taxing part. Showing the progress at first launch is where the need to restructure has reached a tipping point. Restructuring a code base is the major mental effort.

The major roadblock I've encountered is the need to have all images loaded before the Program struct can be fully constructed.

This is a problem, since to indicate progress, I need to render to the screen. However, all the screen rendering methods (specifically relating to the infobar) are on Program.

workarounds or solutions

The following remedies may overlap to fix this problem.

  1. Start out with no images (empty vec), and switch to a loading screen Mode. Then switch to Normal mode once first batch of images is loaded.

  2. Move some of the rendering methods to the screen struct.

    • Specialize some of the infobars to take ui_state as and argument, and others not (like loading mode)
gurgalex commented 5 years ago

@Davejkane

I made rendering the infobar not care about the mode it is in. Mode seemed to be used as a sort of theme for the infobar, so I gave it the proper argument name in the infobar method.

Infobar is now on the Screen struct, as I needed to render to the infobar before the program struct could be created. Specifically while in Program::init

It still seems like having a Loading mode could work. That could be beneficial in being able to cancel a long-running folder scan.

however, the complication in switching from Command -> Loading -> Command is what led me to this possibly hacky solution.

gurgalex commented 5 years ago

You're right, it doesn't work in Mac OS with the current setup. Strange. Wonder why present is not showing the window. I'll look into that because local print statements say the image scanning is working incrementally.

Here's screenshots of the progress indicator.

Screenshot from 2019-06-17 08-03-40 Screenshot from 2019-06-17 08-03-33

gurgalex commented 5 years ago

Weird, seems I'll need to poll for events before the window will show up. Why this works at all on Linux is a mystery. A separate loading mode it is then! https://stackoverflow.com/questions/37848065/sdl-window-not-showing-up-at-all?rq=1

Davejkane commented 5 years ago

@gurgalex yeah I was wondering if it was OS specific. Good luck, sounds a little tricky.

gurgalex commented 5 years ago

@Davejkane I got the window to popup on Mac OS.

Slight issue you may see is a black background when the scanning is in progress. Seems to not happen all the time though.

I need to rework the max viewable images functionality, so that the glob only ever collects that many images. Currently, it says it matched 100000 images when the user only asked for say 5. The UI will then show 1 of 5 images like it should though.

gurgalex commented 5 years ago

Now, the only thing left is to replace setting an artificial length with physically truncating the images vector.

Scanning all the images of the glob only to only use 1% of them doesn't make much sense.

Plus, this reducing the amount of code needing to be maintained.

gurgalex commented 5 years ago

@Davejkane Ready for review again.

gurgalex commented 5 years ago

Alright, It's definitely Mac specific then. Windows and Linux are tested as working.

I'll report back later today.

gurgalex commented 5 years ago

@Davejkane I added a delay after consuming an event to give the window some time to draw. I also strangely needed to allow a for loop that doesn't loop for the window to appear.

I'm unsure if the 100ms delay is enough.

An issue now is that the rainbow spinner appears while the window is present, because the window isn't responding to any events. May be an issue, but the scanning is taking place with UI feedback.

gurgalex commented 5 years ago

Screen Shot 2019-06-19 at 11 43 23 AM 1

gurgalex commented 5 years ago

It does seem the ideal solution is to keep the event loop always handling events regardless of what processing we are doing (another thread).

Hmm, SDL2 had support for registering custom events: https://rust-sdl2.github.io/rust-sdl2/sdl2/struct.EventSubsystem.html#method.push_custom_event

Davejkane commented 5 years ago

@gurgalex just ping me when you're ready for me to have another look.

gurgalex commented 5 years ago

@Davejkane Ready for another look

Davejkane commented 5 years ago

@gurgalex It's definitely looking better. The window builds and I get the progress bar. Weird thing is the back ground keeps switching between black and the grey colour we set as the default background colour. Any idea why that would be?

gurgalex commented 5 years ago

@Davejkane Not sure why it's switching background colors. I tried to fix that, by clearing the screen with a gray background,but still the black screen shows.

I'll look into it more this weekend.

Davejkane commented 5 years ago

I really appreciate the effort man, I know this issue isn't easy.

gurgalex commented 5 years ago

Let's see, New plan for this week.

Make updating state not block getting OS events.

For each frame

  1. Gather any pending events with EventPump::poll_event()
  2. Update state a. From gathered events b. From long-running background tasks (such as long-running glob)
  3. Render state
  4. sleep for remaining frame time.

This hopefully will remove the black screen by not blocking events and rendering. Mode will no longer block event iteration with long running input gathering operations.

Inspiration from https://gameprogrammingpatterns.com/game-loop.html

Davejkane commented 5 years ago

Sounds like a solid idea to me

Davejkane commented 5 years ago

@gurgalex, any movement on this?

gurgalex commented 5 years ago

@Davejkane Sorry, the research project at university requires a lot of my time as of recently. I'd estimate 1-2 weeks is when I'll be available again.

Davejkane commented 5 years ago

@gurgalex, no problem, thanks for update. Best of luck with your research project.