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

lighttable: make lighttable faster on High DPI screen. #1973

Closed
wants to merge 6 commits into
base: master
from

Conversation

Projects
None yet
7 participants
@TurboGit
Copy link
Member

TurboGit commented Jan 5, 2019

Maybe mouse_moved events are now filtered out (some measurement shows that
only 10% of the mouse_move are actually handled).

This is WIP, to be tested on HDPI screen (4k & 5k). This breaks the zoomable lighttable rating for
now.

@TurboGit TurboGit self-assigned this Jan 5, 2019

@TurboGit TurboGit added this to the 2.8 milestone Jan 5, 2019

@TurboGit TurboGit referenced this pull request Jan 5, 2019

Closed

faster lighttable #1813

@cryptomilk

This comment has been minimized.

Copy link
Contributor

cryptomilk commented Jan 5, 2019

It is not perfect yet, but definitely an improvement!

@TurboGit

This comment has been minimized.

Copy link
Member

TurboGit commented Jan 5, 2019

A bit vague. Was it on a 4k screen? Was it quite unusable before and do you feel that with this patch it makes the lightable bearable or even usable? Somehow as fast as on a full HD screen?

@TurboGit

This comment has been minimized.

Copy link
Member

TurboGit commented Jan 5, 2019

Here report that there is no change:
https://redmine.darktable.org/issues/10764#note-18

That's my second attempt, looks like it fails again :)

Maybe there is some other events that take CPU cycle... I don't own a 4k display, so this will probably have to wait.

@TurboGit TurboGit changed the title WIP - lighttable: make lighttable faster on large DPI screen. WIP - lighttable: make lighttable faster on High DPI screen. Jan 5, 2019

@TurboGit TurboGit force-pushed the faster-lighttable-2 branch from 68a1d6d to d65f0e4 Jan 5, 2019

@TurboGit

This comment has been minimized.

Copy link
Member

TurboGit commented Jan 5, 2019

@cryptomilk : can you test the new version I just pushed? Thanks.

@TurboGit TurboGit force-pushed the faster-lighttable-2 branch from d65f0e4 to f84498e Jan 5, 2019

@cryptomilk

This comment has been minimized.

Copy link
Contributor

cryptomilk commented Jan 5, 2019

  • Yes, I have a 4K display
  • Yes, this is a small improvement but not as snappy as it could be.
  • No, it is not perfect yet.
@cryptomilk

This comment has been minimized.

Copy link
Contributor

cryptomilk commented Jan 5, 2019

2b24086bdcae7b9a988f081ea7825862ee22ea75 seems to work!

@TurboGit

This comment has been minimized.

Copy link
Member

TurboGit commented Jan 5, 2019

Ah ah... Is that fast now? How usable?

@cryptomilk

This comment has been minimized.

Copy link
Contributor

cryptomilk commented Jan 5, 2019

Moving the mouse is fast, the selection follows instantly However scrolling is broken with 2b24086bdcae7b9a988f081ea7825862ee22ea75. Only two thumbnails are changing. If I scroll more it is refreshed after some time ...

@TurboGit

This comment has been minimized.

Copy link
Member

TurboGit commented Jan 5, 2019

Right, this is fixed in the new version, I'll push now.

@TurboGit TurboGit force-pushed the faster-lighttable-2 branch 2 times, most recently from 5f287e9 to f2cbf52 Jan 5, 2019

@TurboGit

This comment has been minimized.

Copy link
Member

TurboGit commented Jan 5, 2019

I've just pushed a new version that should fix most issues and handle also the zoomable lighttable.

@cryptomilk

This comment has been minimized.

Copy link
Contributor

cryptomilk commented Jan 5, 2019

Now scrolling works again but the borders for the thumbnails are gone :-)

@cryptomilk

This comment has been minimized.

Copy link
Contributor

cryptomilk commented Jan 5, 2019

Scrolling works, it is super fast but the borders of the thumbnails are gone, we are close :-)

@TurboGit

This comment has been minimized.

Copy link
Member

TurboGit commented Jan 5, 2019

What do you mean by borders of the thumbs? Can you send a screenshot? Thanks.

@cryptomilk

This comment has been minimized.

Copy link
Contributor

cryptomilk commented Jan 5, 2019

@aurelienpierre

This comment has been minimized.

Copy link
Contributor

aurelienpierre commented Jan 5, 2019

Sounds like a CSS rule gone.

@aurelienpierre

This comment has been minimized.

Copy link
Contributor

aurelienpierre commented Jan 5, 2019

Wow ! Just tested, indeed the borders are gone in the lighttable (not the zoomable version though), but that's some blazing speed-up !

@TurboGit

This comment has been minimized.

Copy link
Member

TurboGit commented Jan 5, 2019

Are the borders there during first display and disappear after? Or are they never there? I cannot reproduce that. But this will get fixed and should not degrade the perfs.

@TurboGit TurboGit force-pushed the faster-lighttable-2 branch from f2cbf52 to 14e6587 Jan 5, 2019

@cryptomilk

This comment has been minimized.

Copy link
Contributor

cryptomilk commented Jan 5, 2019

They are gone on startup.

@cryptomilk

This comment has been minimized.

Copy link
Contributor

cryptomilk commented Jan 5, 2019

29caefec7d8d83ce424cfaf3fc35104f90e8a6b9 doesn't fix it ... :-)

@aurelienpierre

This comment has been minimized.

Copy link
Contributor

aurelienpierre commented Jan 7, 2019

yes the lag is there because when reordering we need to redraw everything. So I do expect some lag for some actions and have this at least "usable" in 4k or 5k display.

The lag was on HD 1080 screen. I did not have it before (even on 4K).

@TurboGit

This comment has been minimized.

Copy link
Member

TurboGit commented Jan 7, 2019

@aurelienpierre : thanks I can reproduce the redraw issue with keyboard. will fix.

@TurboGit TurboGit force-pushed the faster-lighttable-2 branch from 2c894c2 to 82a0664 Jan 7, 2019

@TurboGit TurboGit changed the title WIP - lighttable: make lighttable faster on High DPI screen. lighttable: make lighttable faster on High DPI screen. Jan 7, 2019

@TurboGit

This comment has been minimized.

Copy link
Member

TurboGit commented Jan 7, 2019

I consider this last version as usable and should be working fine. I have made many tests. Please try it and report issues. Again, this is a very delicate part with many many possible interactions.

@cryptomilk

This comment has been minimized.

Copy link
Contributor

cryptomilk commented Jan 7, 2019

@TurboGit I will test this asap, probably tomorrow morning. I have to edit some pictures.

@aurelienpierre

This comment has been minimized.

Copy link
Contributor

aurelienpierre commented Jan 8, 2019

Ok that fixes it.

Still broken:

  • rating from keyboard in file manager
  • browsing with arrow keys in zoomable lighttable
  • zooming from keyboard arrows in file manager (after you have clicked on the zoom slider)
  • thumbnails borders in file manager (they work in the zoomable lighttable, for some reason)
@jhmos

This comment has been minimized.

Copy link

jhmos commented Jan 8, 2019

Thanks TurboGit. Happy to say the changes have made it usable on a 5K iMac too. Took me a while to create the build environment, but I can now do some testing for Mac OS.

@TurboGit

This comment has been minimized.

Copy link
Member

TurboGit commented Jan 8, 2019

@aurelienpierre : I can reproduce the first two points. Not the others.

@nilsholle

This comment has been minimized.

Copy link

nilsholle commented Jan 8, 2019

Are the changes already implemented for the thumbnail view at the bottom of the darkroom? Feel like that is still a little slow.

@TurboGit

This comment has been minimized.

Copy link
Member

TurboGit commented Jan 8, 2019

No, for now the filmstrip is still slow. This is next step.

TurboGit added some commits Jan 7, 2019

lighttable: make lighttable faster and usable on High DPI screen.
The lighttable was quite slow due to lot of expose events being
sent (one expose event for each mouse move). And in this case a
full redraw of the lighttable plus the thumbs was done. This
resulted in darktable using all the CPU.

Many mouse_moved events are now filtered out (some mesurement shows that
only 15% of the mouse_move are actually handled). And moreover in the
majority of case only some thumbs are refreshed instead of the whole
lighttable. Some events still need a full refresh:

   - showing/hiding the side panels
   - entering/leaving the full preview mode
   - selecting a picture (others being deselected)
   - changing the starts or reject flag
   - scrolling top/bottom with the keyboard or the mouse
   - changing the zoom level (mouse scroll or from keyboard)
   - when creating a duplicate
   - changing the filtering
   - changing the collection

With the patch the lighttable should be usable on 4k and 5k screens.

Also, this work has fixed some issues:

- going up when on the first row was moving the lighttable but did not
  move the selection to the thumb on the new displayed row.

- with the keyboard, when selecting a picture (enter) and shift-move
  to select other pictures, the initial selected picture was indeed
  not being selected.
filmstrip: make filmstrip faster and usable on High DPI screen.
The filstrip was quite slow due to lot of expose events being
sent (one expose event for each mouse move). And in this case a
full redraw of the filmstrip plus the thumbs was done. This
resulted in darktable using all the CPU.

Many mouse_moved events are now filtered out (some mesurement shows that
only 15% of the mouse_move are actually handled). And moreover in the
majority of case only some thumbs are refreshed instead of the whole
filmstrip. Some events still need a full refresh:

   - showing/hiding the side panels
   - selecting a picture (others being deselected)
   - changing the starts or reject flag
   - scrolling with the keyboard or the mouse
   - changing the size of the filmstrip
   - when creating a duplicate
   - changing the filtering
   - changing the collection

With the patch the filmstrip should be usable on 4k and 5k screens.

@TurboGit TurboGit force-pushed the faster-lighttable-2 branch from a0c8aa4 to 401c292 Jan 8, 2019

@TurboGit

This comment has been minimized.

Copy link
Member

TurboGit commented Jan 8, 2019

@nilsholle : the filmstrip should now be fast too.

@cryptomilk

This comment has been minimized.

Copy link
Contributor

cryptomilk commented Jan 9, 2019

This looks good, I didn't find any issue when testing.

@TurboGit

This comment has been minimized.

Copy link
Member

TurboGit commented Jan 9, 2019

@cryptomilk : just checking... you've tested the last version with the filmstrip?

@cryptomilk

This comment has been minimized.

Copy link
Contributor

cryptomilk commented Jan 9, 2019

Yes, I did

@PaoloAst

This comment has been minimized.

Copy link
Contributor

PaoloAst commented Jan 9, 2019

I've tested this branch and, for me, it seems working fine. I didn't find any issue.

@homer3018

This comment has been minimized.

Copy link

homer3018 commented Jan 9, 2019

Hi guys,

I'd be happy to assist and do some further testing on my newly bought iMac 5k, but as I'm new to mac I have some (possibly dumb) questions like is it ok to have the regular 2.6 DT installed along with the dev version ? If someone would be kind enough to guide me through that that'd be great. Once up and running I hope I can help further with some coding but we'll see to that at a later stage :)
Thanks and keep up the good work !

@TurboGit

This comment has been minimized.

Copy link
Member

TurboGit commented Jan 9, 2019

@homer3018: this is now merged. I cannot help you with building on MacOS. You'll probably get helps on the mailing-list.

@TurboGit TurboGit closed this Jan 9, 2019

@homer3018

This comment has been minimized.

Copy link

homer3018 commented Jan 9, 2019

All good Pascal, thanks for assisting.

@TurboGit TurboGit deleted the faster-lighttable-2 branch Jan 11, 2019

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