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

reduce GUI lag/stuttering by eliminating unnecessary work #18132

Merged
merged 5 commits into from
Jan 22, 2025

Conversation

ralfbrown
Copy link
Collaborator

Whenever a widget is refreshed in darkroom view, the entire window is redrawn and a lot of other activity is kicked off, including running the coordinate transform chain (which gets very slow if there are any active instances of liquify) multiple times. This PR reworks things (short of the complete rearchitecting of expose() which is actually needed) to reduce the number of calls for transforms and the amount of work done in mouse-motion callbacks.

The latter also fixes a subtle UI bug -- when modules have a drawn mask and the shapes display is enabled, but the module itself is disabled, the mouse still interacts with the (invisible) drawn shapes. This leads to surprises such as a scroll or drag resizing/moving a shape rather
than zooming/panning the image. It may be worth cherry-picking 648a74b for 5.0.1.

@ralfbrown ralfbrown added scope: UI user interface and interactions scope: performance doing everything the same but faster labels Jan 1, 2025
@TurboGit TurboGit added this to the 5.2 milestone Jan 1, 2025
@TurboGit
Copy link
Member

@ralfbrown : A conflict resolution is needed. TIA.

Don't call dt_dev_get_pointer_zoom_pos (which calls
dt_dev_distort_transform, which can take 100+ms when a liquify
instance is active with large areas being distorted) on mouse activity
in the darkroom center image unless actually needed.

Avoid calling distort transform from the darkroom view expose()
function unless invoking a module post-expose callback which needs the
transformed mouse pointer position.

Reduce the number of distort_transform calls generated by
dt_view_paint_surface (called by expose) from two to one.

The overall result is much smoother mouse movement and scrolling when
a module with a slow distort_transform() function is enabled.
This reduces overhead and thus lag/stuttering when moving the mouse
across the center image in darkroom (especially for the liquify
module, which needs to compute the distortion map *every* time).

It also fixes a subtle UI bug -- when modules have a drawn mask and
the shapes display is enabled, but the module itself is disabled, the
mouse still interacts with the (invisible) drawn shapes.  This leads
to surprises such as a scroll or drag resizing/moving a shape rather
than zooming/panning the image.
@ralfbrown
Copy link
Collaborator Author

Conflicts resolved.

Copy link
Member

@TurboGit TurboGit left a comment

Choose a reason for hiding this comment

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

There is an issue, to reproduce:

  • Open a fresh edit
  • Enable Liquify
  • Select a single node
  • Fly over the picture, the shape is not visible
  • Click, you'll see that a shape is added in Liquify GUI counter

@TurboGit
Copy link
Member

Thanks for fixing the issue, but sorry still an issue with this PR. Again to reproduce, use Liquify.

  • Open Liquify
  • Place a node or curve
  • See that the image jump around
  • Select any node and change them, see that image jump

(not tested with other module, but at this this UI effect is very disturbing)

@ralfbrown
Copy link
Collaborator Author

The jumping around is a separate issue that has existed for a long time (probably since liquify was first added) - the code which computes the viewport runs the entire distort_transform chain, and if there is a warp at the point it uses for the center, the entire viewport gets moved.... The fix is to have a separate transform chain which ignores local distortions, I wrote draft code for it, but haven't gotten it to work without issues yet.

@TurboGit
Copy link
Member

The jumping around is a separate issue that has existed for a long time (probably since liquify was first added) - the code which computes the viewport runs the entire distort_transform chain, and if there is a warp at the point it uses for the center, the entire viewport gets moved....

I see, indeed... It seems easier to reproduce when the computer CPU is loaded which was the case when I tested the PR.

Copy link
Member

@TurboGit TurboGit left a comment

Choose a reason for hiding this comment

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

Let's move forward, seems ok to me.

@TurboGit TurboGit merged commit d4b5da0 into darktable-org:master Jan 22, 2025
6 checks passed
@ralfbrown ralfbrown deleted the ptr_zoom_pos branch January 22, 2025 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: performance doing everything the same but faster scope: UI user interface and interactions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants