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

[FR] improve responsiveness of mask manipulation #18180

Closed
ralfbrown opened this issue Jan 9, 2025 · 5 comments · Fixed by #18195
Closed

[FR] improve responsiveness of mask manipulation #18180

ralfbrown opened this issue Jan 9, 2025 · 5 comments · Fixed by #18195
Labels
feature: enhancement current features to improve scope: performance doing everything the same but faster

Comments

@ralfbrown
Copy link
Collaborator

Is your feature request related to a problem? Please describe.

Nicholas from "a dabble in photography" demonstrates how manipulating masks can become very laggy when masking a compute-intensive module on a slow(ish) machine with hi-res screen. https://www.youtube.com/watch?v=Aqu3ULnYugw

Describe the solution you'd like

When a module is expanded and has focus, we should make sure that the module's unmasked output is cached, so that we don't have to re-run the module for every change in masking. The un-masked output is simply the module's output with masking turned off.

I anticipate that some change to the blendif code will be required to have it check the cache before running the module to get its output, and to add that output to the cache if it wasn't already cached. This entry should be marked as important so that it stays in cache until the module loses focus or its (non-blending) settings change.

@ralfbrown ralfbrown added feature: enhancement current features to improve scope: performance doing everything the same but faster labels Jan 9, 2025
@TurboGit
Copy link
Member

When a module is expanded and has focus, we should make sure that the module's unmasked output is cached, so that we don't have to re-run the module for every change in masking.

Sure, but we still have to run all module above in the pipeline.

@ralfbrown
Copy link
Collaborator Author

Sure, but we still have to run all module above in the pipeline.

That's not the case when showing the mask in yellow, so adjusting shapes/parameters under that condition should be instantaneous, which the linked video demonstrates currently isn't true for a slow module.

@jenshannoschwalm
Copy link
Collaborator

I did some testing on this already and if you haven't started working on this i would implement that.

Not exactly as you suggested - i would propose a simple single-line proxycache for module-in-focus holding processed data. This would just be some wrapping of module->process() and module->process_cl() in pixelpipe_hb and thus would be easy to test but still pretty efficient. What would you think?

@TurboGit
Copy link
Member

Sure, but we still have to run all module above in the pipeline.

That's not the case when showing the mask in yellow, so adjusting shapes/parameters under that condition should be instantaneous, which the linked video demonstrates currently isn't true for a slow module.

Ah sorry I was thinking about the drawn mask shapes.

@ralfbrown
Copy link
Collaborator Author

@jenshannoschwalm Sure, go ahead. I have several other items on my queue before I'd get to this, which is why I posted it as a FR rather than just starting to work on it.

jenshannoschwalm added a commit to jenshannoschwalm/darktable that referenced this issue Jan 12, 2025
The pixelpipe cache offers *input* data for a module to be fully processed if all parameters including
blending stuff match.

This commit implements a single line cache only effective for a module being in focus *and* doing some blending.
Data are possibly read instead of module->process/_cl, if not found they are written after that.

colorspace correction and blending will be processed as before.

Overall this avoids the possibly costly processing of the module if in focus and response to changes in blending
are much faster.

See darktable-org#18180
jenshannoschwalm added a commit to jenshannoschwalm/darktable that referenced this issue Jan 13, 2025
The pixelpipe cache offers *input* data for a module to be fully processed if all parameters including
blending stuff match.

This commit implements a single line cache only effective for a module being in focus *and* doing some blending.
Data are possibly read instead of module->process/_cl, if not found they are written after that.

colorspace correction and blending will be processed as before.

Overall this avoids the possibly costly processing of the module if in focus and response to changes in blending
are much faster.

See darktable-org#18180
jenshannoschwalm added a commit to jenshannoschwalm/darktable that referenced this issue Jan 14, 2025
The standard pixelpipe cache offers *input* data for a module to be fully processed if all parameters including
blending stuff match.

This commit implements a single line cache per pixelpipe only effective for a module being in focus *and* doing
some blending.

Data are possibly read from this cache instead of the possibly costly module->process/_cl.
Possibly write to that cache if data were not valid before.

Fixes darktable-org#18180
jenshannoschwalm added a commit to jenshannoschwalm/darktable that referenced this issue Jan 22, 2025
The standard pixelpipe cache offers *input* data for a module to be fully processed if all parameters including
blending stuff match.

This commit implements a single line cache per pixelpipe only effective for a module being in focus *and* doing
some blending.

Data are possibly read from this cache instead of the possibly costly module->process/_cl.
Possibly write to that cache if data were not valid before.

Avoid some dereferencing

Fixes darktable-org#18180
TurboGit pushed a commit that referenced this issue Jan 22, 2025
The standard pixelpipe cache offers *input* data for a module to be fully processed if all parameters including
blending stuff match.

This commit implements a single line cache per pixelpipe only effective for a module being in focus *and* doing
some blending.

Data are possibly read from this cache instead of the possibly costly module->process/_cl.
Possibly write to that cache if data were not valid before.

Avoid some dereferencing

Fixes #18180
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: enhancement current features to improve scope: performance doing everything the same but faster
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants