Improve quality of images in notifications (Bug 385097)
ClosedPublic

Authored by rkron on Jan 20 2018, 4:29 PM.

Details

Summary

BUG: 385097
FIXED-IN: 5.12.0

This patch to plasma-workspace is to improve the quality of images in notifications as reported in Bug 385097.

It is still not perfect because KIO::PreviewJob is used to create the thumbnail, and it only produces two sizes, 128px and 256px, so it is likely that a small image will be scaled up, blurring it somewhat.

I took two screenshots of the same Dolphin window. Here is the before, without the patch:

And the after, with the patch:

Better, but still not perfect.

Test Plan

Take screenshots with Spectacle with and without the patched libnotificationshelperplugin library and observe the image quality in the notification. plasmashell must be restarted when the library file is changed for it to take effect.

Diff Detail

Repository
R120 Plasma Workspace
Lint
Lint Skipped
Unit
Unit Tests Skipped
rkron created this revision.Jan 20 2018, 4:29 PM
Restricted Application edited projects, added Plasma; removed Plasma: Workspaces. · View Herald TranscriptJan 20 2018, 4:29 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
rkron requested review of this revision.Jan 20 2018, 4:29 PM
ngraham edited the summary of this revision. (Show Details)Jan 20 2018, 4:30 PM
ngraham added a subscriber: ngraham.
rkron edited the summary of this revision. (Show Details)Jan 20 2018, 4:52 PM
broulik accepted this revision.Jan 20 2018, 7:44 PM
broulik added a subscriber: broulik.

Thanks for looking into this! Do you have commit access?

This revision is now accepted and ready to land.Jan 20 2018, 7:44 PM
rkron added a comment.EditedJan 20 2018, 10:12 PM

@broulik I'm new to phabricator and do not have commit access. Please commit for me. Thanks.

ngraham edited the summary of this revision. (Show Details)Jan 25 2018, 5:01 PM
This revision was automatically updated to reflect the committed changes.
rkron updated this revision to Diff 26060.Jan 27 2018, 2:04 PM

BUG: 385097

I'm sorry for all the trouble, but I found the way to produce really clear previews.

Notification before current patch to libnotificationshelperplugin.so :

Notification after current patch:

I did not fully understand how KIO::PreviewJob worked the first time. Again, I'm sorry for the trouble and I feel like an idiot.

rkron reopened this revision.Jan 27 2018, 2:06 PM

See my comment with the new patch.

This revision is now accepted and ready to land.Jan 27 2018, 2:06 PM

Thanks Randy! Is this in addition to, or instead of your last patch?

rkron added a comment.EditedJan 27 2018, 2:38 PM

@ngraham This is in addition to the last patch, Nate. It needs both, and this diff is off the current state of the repo. When I first looked into this, I tried setting ScaleType but it had no effect. Then I found that the preview image was being scaled based on the small vertical height that was being requested, which led to the first patch. I should have tried setting ScaleType again.

At this point, I think this easiest thing to do is to make a new patch. Thanks!

rkron added a comment.Jan 27 2018, 2:58 PM

@ngraham So should I submit this new patch as a new revision? I'm still new to this.

No worries! This revision is already closed and associated with a commit, so you should open a new revision to hold the new diff. If you're not using Arcanist to generate patches yet, here's the documentation: https://community.kde.org/Infrastructure/Phabricator#Using_Arcanist

It's pretty sweet once you get used to it.

rkron added a comment.Jan 27 2018, 3:08 PM

@ngraham Thanks Nathaniel!

rkron closed this revision.EditedJan 27 2018, 3:09 PM

Submitting new revision.