-
Notifications
You must be signed in to change notification settings - Fork 1.3k
mipmap_cache: respect conf request to always use embedded thumbnail #20103
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
base: master
Are you sure you want to change the base?
Conversation
When conf "plugins/lighttable/thumbnail_raw_min_level" is "never", always use the thumbnail embedded in a raw, even when it is lower resolution the requested mipmap. This fixes a problem particularly visible on high dpi screens with fractional scaling: Due to limitations of GTK3 (which always returns a whole-number from gtk_widget_get_scale_factor), we generate mipmaps larger than the physical display resolution. These can easily exceed the resolution of the embedded JPEG. Fixes darktable-org#19944.
|
Hi @dtorop, it's nice to see your around again :-) I think this may mess up culling (at least for me). I'll test and see what happens. |
|
@wpferguson: Thanks likewise! RE culling: I'm testing now. I think there's a minor problem in |
When prefetching an image, take into account scaling from logical to device pixels when determining requested mipmap size. Prior to this commit, when display scaling > 100%, dt may prefetch a too small mipmap. This results in seeing a "working..." message when moving through images (via preview or normal culling mode), with the initially displayed mipmap soon after replaced by a slightly higher resolution mipmap.
|
Added fix for respecting display scaling in thumbnail prefetch. This is another problem visible when using display scaling. @wpferguson: Does this relate to the problem you are seeing in culling, though? |
|
My situation
With this patch:
My concern is that user's are used to this behavior, since it's been this way for years. Using a scaled up JPG for culling is difficult since you can't tell if it's in focus or not and doing a full size preview to check focus is useless since again it's scaled way up. I can solve my problem by setting CAVEAT: If your embedded JPG is larger than 4K, I guess it's a moot point. So |
|
Should we tag this with |
Probably. It makes the default behavior match the behavior as documented by the config option. But before dt would never upscale a thumb if it could downscale a raw instead, and I understand from @wpferguson that this is a behavior which some (many?) users would prefer. I'm trying to see if dt is a bit more laggy with this PR, when on a 4K display and moving through image previews. I wonder if upscaling a mip6 via Cairo is faster than working with a mip8. |
|
Maybe good to think about ideal behavior rather than extant code vs. documentation. Some useful behaviors:
Are there other scenarios? The current |
I had this when my laptop was 1080p and my external monitor was 2k. I ended up generating cache for both.
My script spits out timing so when set to never 1171 size 3 (pull the embedded jpeg) took 22 seconds, and it took 44 seconds to upscale the 1171 images to size 6. I did cull with them and sometimes displayed full size (upscaled from size 6) and I didn't notice any lag, certainly nothing like building from raw. For reference, building size 6 cache from raw took 470 seconds for 1171 images. EDIT:
I'm not sure they prefer it, but I'm worried they might expect it and be surprised with the change. EDIT 2:
Actually it's online and runs from a post-import-film trigger so that every time I import the thumbtable cache gets built quickly so I can scroll through the thumbtable and get an idea of what I have while the culling cache is being built. Once it's done I can cull at full speed. I don't zoom to full size enough to make worthwhile to build cache for it. |
Good to know about timings. I just tried this PR on a 1920x1080 screen, and it's quite responsive -- but so was dt without this PR on that setup.
Yes, I'm not sure either. In my own case, I prefer using embedded JPEG when culling images, even at fullscreen. I can view images without waiting for pixelpipe, and the embedded JPEGs are decent quality. I believe that the switch to Wayland and its handling of fractional scaling caused a regression for me. On a 4K display, culling images became laggy, as dt was busily running the pixelpipe in the background. It seems like there's a balance, where some users want low latency and some want highest resolution, and ideally dt could accommodate both (with a user-visible conf option). I did notice that with this PR, using ctrl-scroll or ctrl-middle-click to zoom in an image has a change in behavior with default config. Before, dt would run the full pixelpipe at 100% and (after a lag) show 1:1 the detail in the raw file (albeit with the image looking different than a zoomed out view, due to running the pixelpipe). Now, dt will generate a mip8 from the embedded JPEG, and zoom in on that. For me, I can get decent sharpness information even from the embedded JPEG at 100% (or switch to darkroom and see the raw), and appreciate that it is faster not to run the pixelpipe. Others may have a different experience/preference! |
|
Yeah, the main divide is going to be whether the camera's largest embedded preview is anywhere near the monitor's resolution, e.g 4K monitor and a Sony camera from when they didn't embed anything bigger than 1920x1080 (some really old cameras from various manufacturers only had 640x480!) Maybe we could add an "auto" option to "use raw from size", where Auto selects what amounts to the old behavior: a cutoff of the monitor's linear pixel size and no upscaling from mipmaps. |
|
To document the changed behavior for the conf setting Prior Behavior
Behavior with this PRChanged behavior is emphasized.
RecommendationIf you previously used the
If you previously used the QueryWould it be helpful to add |
Overlapping comments! But yes, that is exactly it! And |
A place this gets a bit tricky is when the user changes the conf value from/to |
|
Just mentioning this, when using either the lua script or dt internal backgriund update thumbs we currently already have thumbs in requested size processed after a short while. If i add a day's shots i just have an espresso and all is good and fast for culling. But "never is never" :-) Dan, nice having you around again ! |
|
@jenshannoschwalm Thank you! For my workflow, I tend to ignore the small thumbs and go through each image via fullscreen preview, and give each image a second or two to rate it. So I'm sensitive to when those are slow to load, hence a true I'm trying to wrap my head around how to discard thumbs in |
|
@dtorop : Nice to see you again :) I also voice my approval for "never is never". I've not looked at the actual rendering when upscaling, but if a user want this then the option is there. |
These replicate the prior behavior of "never" mipmaps for conf "plugins/lighttable/thumbnail_raw_min_level": When generating a mipmap from an altered image, use the embedded JPEG unless the requested mipmap size is greater than the embedded JPEG size. In that case generate the image from running the full pixelpipe. This allows for fast mipmaps, but unlike the "never" behavior, avoids upscaling the embedded JPEG Update the thumbnail discard code to handle switching between auto/never modes. We don't have the metadata for mipmaps to know whether they were/weren't upscaled or generated from a JPEG/RAW. Rather than modify mipmap code to support an edge case, use a heuristic: All semi-recent cameras appear to have embedded JPEG of at least mip3 quality. So on switch between auto/never modes, discarding all mip4 or greater resoluton is sufficient. This code also does the right thing when switching between "always", "small", through "5K" values and "never"/"auto" without needing special case handling.
The discard loop previously skipped the highest resolution mipmap in the range. Prior behavior has been in the code since it was committed in 4f0b668 but looks to be an omission.
|
@TurboGit: Thank you! Yes "never is never" seems like the right formulation. I added a commit to support an Also added a release note to document this. And a small bugfix where the code skipped discarding cached mip8 files during a conf switch. |
Don't need to have bespoke messages for auto/never conf options.
When conf
plugins/lighttable/thumbnail_raw_min_levelisnever, always use the thumbnail embedded in a raw, even when it is lower resolution the requested mipmap.This fixes a problem particularly visible on high dpi screens with fractional scaling: Due to limitations of GTK3 (which always returns a whole-number from gtk_widget_get_scale_factor), we generate mipmaps larger than the physical display resolution. These can easily exceed the resolution of the embedded JPEG. This problem is more visible now that darktable can run on Wayland, which allows fractional scaling.
Fixes #19944.