Differential D10187

Update view after changing rendering intent config
ClosedPublic
Actions

Authored by rkflx on Jan 29 2018, 10:26 PM.

Details

Reviewers
ngraham
muhlenpfordt
Commits
R260:899423ad4161: Update view after changing rendering intent config
Summary

This is a follow-up patch to D10076, so after changing the rendering
intent in Gwenview's config dialog the displayed image is updated
immediately after clicking on OK or Apply. Before, it was
necessary to Reload the image manually or pan around.

Currently this is only implemented for raster images. In case someone
figures out colour management for SVGs, this patch should be extended to
SVGs too.

Depends on D10076: Add a user-facing control to choose the ICC color rendering intent.

CCBUG: 359909

Test Plan

Set up an ICC profile as described in D10076#197598 and open an
out-of-gamut image. Change the rendering intent in Gwenview's config
dialog, click Apply and observe that the image's colours change
immediately. Saving to the config file still works.

Diff Detail

Repository
R260 Gwenview
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
rkflx requested review of this revision.Jan 29 2018, 10:26 PM
rkflx created this revision.
rkflx edited the summary of this revision. (Show Details)Jan 29 2018, 10:34 PM
ngraham accepted this revision.Jan 30 2018, 1:57 AM

Lovely; works perfectly, and the code changes seem sane and sensible. Thanks so much, Henrik!

This revision is now accepted and ready to land.Jan 30 2018, 1:57 AM
muhlenpfordt added a comment.Jan 30 2018, 9:40 AM

I tried to setup color profiles, but I think my monitor does not support this. Only printers show up in kcm_colord and colormgr.
Code looks ok to me and updateBuffer() is called on Ok and Apply, so it seems to work. :)

rkflx edited the summary of this revision. (Show Details)Jan 30 2018, 11:54 PM

Thanks for testing, everyone.

In D10187#198041, @muhlenpfordt wrote:

I tried to setup color profiles, but I think my monitor does not support this. Only printers show up in kcm_colord and colormgr.

That's strange. As far as I've understood, the monitor only has to provide its EDID, with the rest done in software and on the GPU (at least for regular consumer hardware). Sounds more like a setup problem or outdated software…

Maybe you have to relogin in case you newly installed colord? Otherwise, I only found https://bugzilla.gnome.org/show_bug.cgi?id=753291.

Anyway, I'll commit for now. Normally filing a bug for the monitor not showing up would be good, but currently I'm not sure where the best place for that would be…

Closed by commit R260:899423ad4161: Update view after changing rendering intent config (authored by rkflx). · Explain WhyJan 30 2018, 11:57 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 26229

View Options

lib/documentview/rasterimageview.h

Loading...
View Options

lib/documentview/rasterimageview.cpp

Loading...
View Options

lib/documentview/rasterimageviewadapter.cpp

Loading...
View Options

lib/renderingintent.h

Loading...