Skip to content

darkroom: reduce filtering sooner for when zoomed in#9869

Merged
TurboGit merged 1 commit intodarktable-org:masterfrom
dtorop:darkroom-closeup-scale-speed
Aug 26, 2021
Merged

darkroom: reduce filtering sooner for when zoomed in#9869
TurboGit merged 1 commit intodarktable-org:masterfrom
dtorop:darkroom-closeup-scale-speed

Conversation

@dtorop
Copy link
Copy Markdown
Contributor

@dtorop dtorop commented Aug 23, 2021

In debugging what @MStraeten noticed in #9835 (comment)

unfortunately moving the live-picker results in a reprocessing of the preview image each time the picker is set, so it comes with a price - depending on the used hardware and the pixelpipe this can be very time consuming (a 'working...' on picking a color for setting a parametric mask is annoying)

it seems that the darkroom center view should fall back to fast filtering sooner than it current does.

Currently CAIRO_FILTER_FAST is enabled at > 100% scale for lodpi screens and at > 400% (e.g. 800%) scale for hidpi screens. This makes updates at 400% on hidpi particularly slow.

This PR enables CAIRO_FILTER_FAST at >= 100% for lodpi and at >= 200% for hidpi. This noticeably speeds up center view refreshes at 400% for hidpi.

This PR also uses fewer function calls/mutexes as it takes advantage of the caller already knowing the current closeup value, and that being enough to determine when to go to lower resolution.

Note that CAIRO_FILTER_FAST appears to be capable of deciding to use no scaling at 1:1. See https://github.com/freedesktop/cairo/blob/923715f2e9acec31f6af406f39e14c36f9f7365c/src/cairo-pattern.c#L3424-L3436.

This PR fixes a bug by which the second window reduced filtering depending on the zoom of the primary window.

@johnny-bit johnny-bit added scope: performance doing everything the same but faster scope: UI user interface and interactions bugfix pull request fixing a bug labels Aug 24, 2021
@johnny-bit johnny-bit added this to the 3.8 milestone Aug 24, 2021
Comment thread src/views/darkroom.c Outdated
@dtorop dtorop force-pushed the darkroom-closeup-scale-speed branch from 16739e1 to df44371 Compare August 24, 2021 21:43
Turn on CAIRO_FILTER_FAST at >= 200% scale for hidpi screens. Current
code turns on CAIRO_FILTER_FAST at >= 800% for hidpi.

This speeds up center view drawing particularly at 400% on hidpi
screens. Cairo already detects 1:1 rendering and therefore used a fast
nearest neighbor path for 200% zoom.

Behavior should be unchanged for lodpi screens, where
CAIRO_FILTER_FAST was turned on at > 100% (with Cairo already to use a
fast path at 100%) and is now turned on at >= 100%.

Also, 2nd window filtering was determined based on the primary
window's zoom settings. This is corrected in this version.

The filtering level helper functions now use zoom/closeup values from
their callers, hence no need to fetch them again and have extra
mutexes.

Note that this works with hidpi when 2x2 blocks of device pixels act as
1x1 logical pixel. The whole zoom code gets flaky if > 2x2 blocks of
device pixels act as a 1x1 device pixel.
@dtorop dtorop force-pushed the darkroom-closeup-scale-speed branch from df44371 to 284ed5c Compare August 24, 2021 21:47
@dtorop
Copy link
Copy Markdown
Contributor Author

dtorop commented Aug 24, 2021

This fix should be fixed now. I updated to PR comment as well, and merged the multiple commits down to one with a (finally, I think) correct commit message.

Essence:

  • Behavior should be unchanged for lodpi
  • Should be fixed for hidpi
  • Should be fixed for 2nd window

Comment thread src/views/darkroom.c
// is represented on screen by multiple pixels we want to disable any cairo filter
// which could only blur or smooth the output.

if(scale / darktable.gui->ppd > 1.0)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm still not sure about this. The gui->ppd is controlling the down-sampling so it seems correct to control the behavior based on this here no? See option down-sampling in the prefs which can be original, 1/2, 1/3 or 1/4.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

gui->ppd is already controlling the closeup value, which controls the dt_dev_get_zoom_scale() result above... So ppd is currently double-applied: once in setting up closeup, and once in _get_filtering_level(). For a ppd of 1 (lodpi), that is fine (a NOP). For a ppd of 2, though, fast filtering kicks in one notch more (800%) than what extant code wants.

Also, the > 1.0 should be >= 1.0 (or equivalent with epsilon). For 1:1, Cairo does the right thing, regardless of the current filter, but above that things go awry, which is why hidpi fast filtering is currently is OK at 200%, slow at 400%, and OK at >= 800%.

Example code which already sets closeup based on ppd:

darktable/src/views/darkroom.c

Lines 3674 to 3698 in 52f261b

// for 200% zoom or more we want pixel doubling instead of interpolation
if(scale > 15.9999f / ppd)
{
scale = dt_dev_get_zoom_scale(dev, DT_ZOOM_1, 1.0, 0);
zoom = DT_ZOOM_1;
closeup = low_ppd ? 4 : 3;
}
else if(scale > 7.9999f / ppd)
{
scale = dt_dev_get_zoom_scale(dev, DT_ZOOM_1, 1.0, 0);
zoom = DT_ZOOM_1;
closeup = low_ppd ? 3 : 2;
}
else if(scale > 3.9999f / ppd)
{
scale = dt_dev_get_zoom_scale(dev, DT_ZOOM_1, 1.0, 0);
zoom = DT_ZOOM_1;
closeup = low_ppd ? 2 : 1;
}
else if(scale > 1.9999f / ppd)
{
scale = dt_dev_get_zoom_scale(dev, DT_ZOOM_1, 1.0, 0);
zoom = DT_ZOOM_1;
if(low_ppd) closeup = 1;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ok, thanks for clarifying. So this sounds good now. I'll merge right away to ensure lot of field testing as this is a really delicate area and I'm not sure to have a complete view on the possible consequences.

Copy link
Copy Markdown
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.

Thanks !

@TurboGit TurboGit merged commit 8e136ef into darktable-org:master Aug 26, 2021
@dtorop dtorop deleted the darkroom-closeup-scale-speed branch August 27, 2021 01:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix pull request fixing a bug 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.

3 participants