SpectacleProject
ActivePublic

Recent Activity

Fri, Apr 12

broulik closed D20463: Add DesktopEntry to notifyrc.
Fri, Apr 12, 2:14 PM · Spectacle
davidre added a comment to D19591: Add Compression Quality slider for lossy formats.

P370 (Warning may write many files)
All judging from file sizes:
JPG/JPEG: Quality has an effect
PIC: compression 0 disables compression (enabled by default?)
PNG: see above
TIF/TIFF: same as PIC
WEBP: only quality seems to have an effect

Fri, Apr 12, 1:10 PM · Spectacle
cfeck added a comment to D19591: Add Compression Quality slider for lossy formats.

JPEG 2000 and WebP are other (somewhat less popular) lossy formats. The Qt format writers switch to lossless mode for quality 100, see https://code.qt.io/cgit/qt/qtimageformats.git/tree/src/plugins/imageformats/webp/qwebphandler.cpp#n251 and https://code.qt.io/cgit/qt/qtimageformats.git/tree/src/plugins/imageformats/jp2/qjp2handler.cpp#n827

Fri, Apr 12, 11:58 AM · Spectacle
nrother added a comment to D19591: Add Compression Quality slider for lossy formats.

I looked into Qt and it actually uses compression. It just falls back to quality if compression is not set. See: https://code.qt.io/cgit/qt/qtbase.git/tree/src/gui/image/qpnghandler.cpp#n1075
I did a quick test P367 and it seems to work. The question now is do we want to special case this for png to not accidentally break another format (would be a quick fix) or to enable it for all formats that support it. In my opinion if we do the latter we should now do testing (maybe automated?) to not run into weird quality/compression interactions for other formats.
@nrother Do you want to fix it for 19.04.1?

Fri, Apr 12, 10:37 AM · Spectacle

Thu, Apr 11

davidre added a comment to D19591: Add Compression Quality slider for lossy formats.

I looked into Qt and it actually uses compression. It just falls back to quality if compression is not set. See: https://code.qt.io/cgit/qt/qtbase.git/tree/src/gui/image/qpnghandler.cpp#n1075
I did a quick test P367 and it seems to work. The question now is do we want to special case this for png to not accidentally break another format (would be a quick fix) or to enable it for all formats that support it. In my opinion if we do the latter we should now do testing (maybe automated?) to not run into weird quality/compression interactions for other formats.
@nrother Do you want to fix it for 19.04.1?

Thu, Apr 11, 2:12 PM · Spectacle
ngraham accepted D20463: Add DesktopEntry to notifyrc.

+1, please push to stable.

Thu, Apr 11, 1:56 PM · Spectacle
broulik requested review of D20463: Add DesktopEntry to notifyrc.
Thu, Apr 11, 10:42 AM · Spectacle

Wed, Apr 3

kdautovic added a comment to D19715: Add option to copy screenshot to clipboard in background mode.

Oooh, yeah that's a bug. I guess we should always add x-kde-force-image-copy to the metadata of a copied screenshot? Wanna do that on the Applications/19.04 branch?

That's where my confusion here and in D20180 is coming from. Isn't it always added unconditionally in  `ExportManager::doCopyToClipboard`? And both the button regardless of quit after copy and spectacle -b -c go through that.  Or am I missing a place where copying is done?
Wed, Apr 3, 8:19 PM · Spectacle
ngraham added inline comments to D19310: [WIP] Port to KGlobalAccel.
Wed, Apr 3, 3:13 PM · Spectacle
davidre added a comment to D19310: [WIP] Port to KGlobalAccel.

Broken Editor:

Wed, Apr 3, 2:35 PM · Spectacle
davidre updated the diff for D19310: [WIP] Port to KGlobalAccel.
  • remove setup shortcuts
Wed, Apr 3, 2:34 PM · Spectacle
davidedmundson added inline comments to D19310: [WIP] Port to KGlobalAccel.
Wed, Apr 3, 2:24 PM · Spectacle
davidre added a comment to D19310: [WIP] Port to KGlobalAccel.

@davidedmundson is /usr/share/kglobalaccel the correct folder?

yes.

@davidre can you put this back to being broken so we know what to fix.

Wed, Apr 3, 2:05 PM · Spectacle
davidedmundson added a comment to D19310: [WIP] Port to KGlobalAccel.

@davidedmundson is /usr/share/kglobalaccel the correct folder?

Wed, Apr 3, 1:59 PM · Spectacle
ngraham added a comment to D19310: [WIP] Port to KGlobalAccel.

Could this be part of the problem?

Wed, Apr 3, 1:45 PM · Spectacle
davidre added a comment to D19591: Add Compression Quality slider for lossy formats.

It seems for the png implementation setQuality also sets the Compression instead of setCompression:
https://bugreports.qt.io/browse/QTBUG-43618

Well that explains it. Unfortunately it seems like the Qt devs don't plan on fixing so I guess it's on us?
Two options that make sense to me here is check if format is png and then either set a sane default value for compression
or take the current quality value and invert it so that higher values for compression quality mean higher png compression.
So setting compression quality to e.g 100 in the UI results in quality being set to 0 for the ImageWriter so that the resulting
image is compressed at level 9 (so the highest level for png).

Wed, Apr 3, 11:08 AM · Spectacle
nrother added a comment to D19591: Add Compression Quality slider for lossy formats.

It seems for the png implementation setQuality also sets the Compression instead of setCompression:
https://bugreports.qt.io/browse/QTBUG-43618

Wed, Apr 3, 10:47 AM · Spectacle
filipf added a comment to D19591: Add Compression Quality slider for lossy formats.

Where is the check that this is done only for lossy formats? Here, PNG is saved uncompressed after this change.

Wed, Apr 3, 10:42 AM · Spectacle
davidre added a comment to D19591: Add Compression Quality slider for lossy formats.

It seems for the png implementation setQuality also sets the Compression instead of setCompression:
https://bugreports.qt.io/browse/QTBUG-43618

Wed, Apr 3, 10:35 AM · Spectacle
nrother added a comment to D19591: Add Compression Quality slider for lossy formats.

Where is the check that this is done only for lossy formats? Here, PNG is saved uncompressed after this change.

Wed, Apr 3, 10:34 AM · Spectacle
cfeck added a comment to D19591: Add Compression Quality slider for lossy formats.

Where is the check that this is done only for lossy formats? Here, PNG is saved uncompressed after this change.

Wed, Apr 3, 10:07 AM · Spectacle

Tue, Apr 2

davidre added a comment to D19715: Add option to copy screenshot to clipboard in background mode.

Oooh, yeah that's a bug. I guess we should always add x-kde-force-image-copy to the metadata of a copied screenshot? Wanna do that on the Applications/19.04 branch?

That's where my confusion here and in D20180 is coming from. Isn't it always added unconditionally in ExportManager::doCopyToClipboard? And both the button regardless of quit after copy and spectacle -b -c go through that. Or am I missing a place where copying is done?

Tue, Apr 2, 6:00 PM · Spectacle
ngraham added a comment to D19715: Add option to copy screenshot to clipboard in background mode.

Testing master using quit after copy works but doing spectacle -b -c does not. Can you confirm?

Tue, Apr 2, 4:11 PM · Spectacle
davidre added a comment to D20180: [RFC] Make notifications responsibility of SpectacleCore.

You're right. There seems to be some magic involved. If I wait 250 ms and directly quit like KSMainWindow did it works. Testing master I also found that spectacle -b -c does not work and if you copy to the clipboard and then exit manually the image doesn't appear Klipper.
Now to figure out the reason

Tue, Apr 2, 7:49 AM · Spectacle
davidre added a comment to D19715: Add option to copy screenshot to clipboard in background mode.

Testing master using quit after copy works but doing spectacle -b -c does not. Can you confirm?

Tue, Apr 2, 7:44 AM · Spectacle

Mon, Apr 1

ngraham requested changes to D20180: [RFC] Make notifications responsibility of SpectacleCore.

Thanks, a nice refactoring. However in testing this broke the "quit after copy" feature; now the image is no longer added to klipper when this setting is used.

Mon, Apr 1, 7:55 PM · Spectacle
broulik closed D20168: Remove "Open" button and make notification clickable instead.
Mon, Apr 1, 6:22 PM · Spectacle
davidre edited reviewers for D20180: [RFC] Make notifications responsibility of SpectacleCore, added: Spectacle; removed: VDG.
Mon, Apr 1, 5:38 PM · Spectacle
davidre requested review of D20180: [RFC] Make notifications responsibility of SpectacleCore.
Mon, Apr 1, 5:37 PM · Spectacle
ngraham accepted D20168: Remove "Open" button and make notification clickable instead.
Mon, Apr 1, 3:35 PM · Spectacle
broulik updated the summary of D20168: Remove "Open" button and make notification clickable instead.
Mon, Apr 1, 2:37 PM · Spectacle
broulik updated the test plan for D20168: Remove "Open" button and make notification clickable instead.
Mon, Apr 1, 2:37 PM · Spectacle
broulik updated the diff for D20168: Remove "Open" button and make notification clickable instead.
  • Rebase
Mon, Apr 1, 2:36 PM · Spectacle
broulik requested review of D20168: Remove "Open" button and make notification clickable instead.
Mon, Apr 1, 2:31 PM · Spectacle

Tue, Mar 26

ngraham added a comment to D19715: Add option to copy screenshot to clipboard in background mode.

Oh haha

Tue, Mar 26, 10:04 PM · Spectacle
pino added a comment to D19715: Add option to copy screenshot to clipboard in background mode.

Broken feature? What doesn't work? I tested it out and it worked fine for me.

Tue, Mar 26, 10:04 PM · Spectacle
ngraham added a comment to D19715: Add option to copy screenshot to clipboard in background mode.

Broken feature? What doesn't work? I tested it out and it worked fine for me.

Tue, Mar 26, 10:02 PM · Spectacle
pino added a comment to D19715: Add option to copy screenshot to clipboard in background mode.

Because of the broken feature and string freezes, the commit was reverted in Applications/19.04, while still part of master (so it will be part of Applications 19.08).

Tue, Mar 26, 9:56 PM · Spectacle
ngraham added inline comments to D19715: Add option to copy screenshot to clipboard in background mode.
Tue, Mar 26, 8:06 PM · Spectacle
cfeck added inline comments to D19715: Add option to copy screenshot to clipboard in background mode.
Tue, Mar 26, 7:58 PM · Spectacle
ngraham closed D19715: Add option to copy screenshot to clipboard in background mode.
Tue, Mar 26, 6:37 PM · Spectacle
ngraham accepted D19715: Add option to copy screenshot to clipboard in background mode.

Thanks, this works great. Spectacle actually does exit correctly after using the -c option, but it just takes it a few seconds because it has to be running while the notification is visible. I think that's fine.

Tue, Mar 26, 6:32 PM · Spectacle
ngraham updated the summary of D19715: Add option to copy screenshot to clipboard in background mode.
Tue, Mar 26, 6:28 PM · Spectacle
ngraham updated the summary of D19715: Add option to copy screenshot to clipboard in background mode.
Tue, Mar 26, 6:16 PM · Spectacle
ngraham updated the summary of D19715: Add option to copy screenshot to clipboard in background mode.
Tue, Mar 26, 6:15 PM · Spectacle

Sun, Mar 24

kdautovic updated the diff for D19715: Add option to copy screenshot to clipboard in background mode.

Use arcanist to get context

Sun, Mar 24, 7:20 PM · Spectacle
ngraham added a comment to D19715: Add option to copy screenshot to clipboard in background mode.

Fair enough. Let's refactor in another revision.

Sun, Mar 24, 7:14 PM · Spectacle
kdautovic added inline comments to D19715: Add option to copy screenshot to clipboard in background mode.
Sun, Mar 24, 7:12 PM · Spectacle
ngraham added inline comments to D19715: Add option to copy screenshot to clipboard in background mode.
Sun, Mar 24, 7:09 PM · Spectacle
kdautovic updated the diff for D19715: Add option to copy screenshot to clipboard in background mode.

Update doCopyToClipboard function with the "x-kde-force-image-copy" mime type hint.

Sun, Mar 24, 6:57 PM · Spectacle