Skip to content
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

Blurred icons #700

Open
crojack opened this issue Sep 9, 2024 · 87 comments
Open

Blurred icons #700

crojack opened this issue Sep 9, 2024 · 87 comments
Labels
question Question about installing, configuring or using Shutter

Comments

@crojack
Copy link

crojack commented Sep 9, 2024

Some of the Shutter icons are crisp and clear and some are blurred. Can you please tell me what I need to change (in the source code?) to fix this problem?

Cheers,

Screenshot_2024-09-09_22-50-51

@crojack crojack added the question Question about installing, configuring or using Shutter label Sep 9, 2024
@Photon89
Copy link
Member

The icons on the left panel are very small pixel graphics (22x22px), those in the menu on the right are vector graphics with 48x48px which could be set to a higher resolution. On the top panel are icons from your icon theme, which obviously have a higher resolution already. We'd need bigger icons for the left panel, I guess. Maybe other icons because the ones we currently have also behave badly on dark GTK themes.

@crojack
Copy link
Author

crojack commented Sep 10, 2024

The icons on the left panel are very small pixel graphics (22x22px), those in the menu on the right are vector graphics with 48x48px which could be set to a higher resolution. On the top panel are icons from your icon theme, which obviously have a higher resolution already. We'd need bigger icons for the left panel, I guess. Maybe other icons because the ones we currently have also behave badly on dark GTK themes.

The icons you see on the left panel are 48X48 vector graphics I created for Shutter just to see if the problem was related to the pixel graphics icons.

The icons on the right panel are original Shutter 48x48 vector graphics icons which should be clear and crisp. And why wouldn't they be when the icons on the top panel from the system theme are also vector icons and have the same 48x48 resolution?

I seems to me that Shutter's annotation editor has a problem with icon resizing.

@crojack
Copy link
Author

crojack commented Sep 10, 2024

Anyone? Any idea why the ToolBarDrawing icons in the Shutter's built in annotation tool are blurred/pixelated? I've tried to replace them with vector icons (svg) and even with high quality png icons but nothing's changed.

@Photon89
Copy link
Member

It's all in /usr/share/shutter/resources/modules/Shutter/Draw/UIManager.pm. We are using this class in line 62: https://docs.gtk.org/gtk3/class.IconFactory.html No idea where the blurriness comes from yet, though...

@Photon89
Copy link
Member

Actually, it says

Creates a new GtkIconSet with pixbuf as the default/fallback source image. If you don’t add any additional GtkIconSource to the icon set, all variants of the icon will be created from pixbuf, using scaling, pixelation, etc. as required to adjust the icon size or make the icon look insensitive/prelighted.

at https://docs.gtk.org/gtk3/ctor.IconSet.new_from_pixbuf.html Possibly this pixelation and scaling adjustments make the icons blurry.

@Photon89
Copy link
Member

Since this class is deprecated, maybe we can switch to https://docs.gtk.org/gtk3/class.IconTheme.html and use add_resource_path to add our custom icons which are not part of GTK themes.

@crojack
Copy link
Author

crojack commented Sep 12, 2024

So, everything is in UIManager.pm. Thanks for your help. :)

@Photon89
Copy link
Member

I think, this won't work for all icons because some of them just aren't available from GTK themes. Like the pixelation or the numbering buttons.

@Photon89
Copy link
Member

Nice! Now we just need to find some highres or vector icons for the various buttons!

@Photon89
Copy link
Member

Creating some nice icons is not a problem at all. I'll do that.

That would be great! In case, you really tackle this, please have a look at #382 first, people have problems with our currently used icons and it would be great, if they could be solved!

The problem is that I am not able to load custom svg (vector) icons, so I'll need your help.

Unfortunately, I'll be super busy at work for at least another week, so I cannot do any research on this topic immediately, but did you try this: #700 (comment) ?

@Photon89
Copy link
Member

Photon89 commented Oct 16, 2024

I am not a programmer and I am pretty clueless when it comes to perl programming.

Sounds familiar, that's how I ended up in the Shutter team. 😄

But when I compile Shutter with the modified UIManager.pl the custom svg icons don't show up.

You could try to add debug output to the line where the icons should be loaded in UIManager.pl, then see if there are any useful warnings or errors around. Something like

print "\n\n\n Here we are trying to load the icons but it does not work. \n\n\n"

But that's just an idea from a person who is clueless when it comes to perl prorgramming himself. Maybe @DarthGandalf has further ideas!

Thanks for spending so much time and effort trying to fix this issue!

P.S.: Really nice icons you created there!

@Photon89
Copy link
Member

To be honest, I liked the colorful icons more, they are visible on both light and dark GTK themes and don't include a white background which looks a bit ugly on dark GTK themes.

Maybe just make the icons for shapes like arrow, ellipse or rectangle red (since this is the default color for them as well) to make the icons well visible on light and dark GTK themes?

@crojack
Copy link
Author

crojack commented Oct 19, 2024

Again, the icons are not the problem, the problem is that we can't load svg icons into the toolbar_drawing without them being blurry and pixelated.

@Photon89
Copy link
Member

Absolutely, I just wanted to comment on the icons as you posted some screenshots!

Regarding the actual issue, did you try adding print lines to find the output lines of interest among the hundreds of irrelevant lines that Shutter prints easier?

@DarthGandalf
Copy link
Member

Looking at this code, I noticed that it uses lots of outdated Gtk functions: GtkUIManager, GtkActionGroup.

Basically, the whole UI of the draw tool need to be replumbed to either GtkBuilder or creating the objects directly in perl. That would remove the limitation you found with svgs

@crojack
Copy link
Author

crojack commented Oct 26, 2024

So, if I understand correctly, that would mean that UIManager.pl is obsolete as well and the whole UI code should be moved...where? DrawingTool.pl?

Why not create a whole new annotation application based on the Shutter's DrawingTool?
We really don't have a real annotation tool for Linux. Neither Shutter's DrawingTool nor Ksnip can draw polygons and bent/arched arrows and lines. MacOS' Preview can do that. Now I have to do more complex annotations in Inkscape, which sucks for many reasons.

@DarthGandalf
Copy link
Member

So, if I understand correctly, that would mean that UIManager.pl is obsolete as well and the whole UI code should be moved...where? DrawingTool.pl?

For example, yes.

In the prototype I started for Rust/Gtk4 port a while ago (and never finished), I've put that to a separate xml file: https://github.com/shutter-project/shutter-rust/blob/master/resources/editorwindow.ui but this XML is a different format from GtkUIManager's one.

Why not create a whole new annotation application based on the Shutter's DrawingTool?

That would be nice

@Photon89
Copy link
Member

Photon89 commented Oct 31, 2024

Woah, nice! I'd like to open the discussion with some feedback from my perspective.

Things I like a lot about the new prototype:

  1. It's a standalone app, a long requested feature. This allows to address Add command line option to start editor directly (was: How to start shutter editor from CLI?) #542, How to invoke the shutter drawing tool directly from command line #579 and [request] Opening from command line straight into edit mode in a maximized Shutter #682.
  2. The image is centered.
  3. The icons look fantastic and many of them should be visible on high res screens and dark backgrounds as well, which allows to address Edit window's toolbar with dark system theme is indiscernible  #367, In Edit, can't make out most icons on dark theme and high res screen #382, Shutter editor icons are almost invisible in dark theme Desktop Environment #578 and Toolbars too small on my screen. #674.
  4. The "Zoom Fit Best" button, very useful!

Things that I'd like to mention to possibly reconsider:

  1. I liked the old toolbar placement more. It clearly separates the three different toolbars (file actions on the top, tool selection on the left, tool customization on the bottom) while in the new prototype tool selection and customization are grouped in one toolbar. The old toolbar placement also saves vertical space (which might be more valuable than horizontal space on wide screen monitors). Finally, currently the two horizontal toolbars have different total widths (the upper one is wider than the lower one).
  2. I liked the fact that the old file actions toolbar used stock GTK icons, because this way the look better integrates into the rest of the desktop. Of course, for actions that don't have suiting stock icons it is necessary to use custom icons, but for common actions I'd rather stick to the stock icons for consistency reasons.
  3. The icon for the automatic numbering tool is currently just a "1" which might be misleading. When I first saw it, I thought that you added a new tool for number fields (similar to text fields). A "1" inside a circle (otherwise keeping the icon's style) would better represent what the tool is doing, in my opinion.
  4. Some of the icons might still be a bit tricky to discern on dark backgrounds, in particular the five tools for drawing (arrow, line etc.). I don't have a good solution though. Maybe make them red instead of black?

Also, a question: the menu bar (File, Edit, Tools, View) is gone now. This removes some export options and the copy/cut/paste options. Also, if it will be a standalone app, it'd need a Help/Info menu, I think. Is it intended or did you just not take care of it so far?

Overall, I really like the new prototype! Thanks for spending so much time and effort working on it!

@DarthGandalf
Copy link
Member

With lots of tools being added, please don't fall into a feature creep mode

@Photon89
Copy link
Member

This looks damn nice! 😃 👍

@DarthGandalf
Copy link
Member

dimming of the area outside of the rectangular selection

When using such tool, it's often useful to highlight several regions - meaning, to dim everything except several regions.

If you apply it one region at a time, the result would be different

@crojack
Copy link
Author

crojack commented Dec 20, 2024

dimming of the area outside of the rectangular selection

When using such tool, it's often useful to highlight several regions - meaning, to dim everything except several regions.

If you apply it one region at a time, the result would be different

That is a plan. You draw as many rectangles as you want and everything outside them is dimmed. I think that's a cool feature.

@DarthGandalf
Copy link
Member

Not so sure these news is very good :) Do you mean that for some reason it's slow on X11?

@crojack
Copy link
Author

crojack commented Dec 20, 2024

I wasn't sure it would run on Wayland at all, that's why :) . But, no, it's not slow on Xorg when drawing on downscaled 4k images on my slow computer. Drawing on 8k is too slow and probably requires a dedicated GPU.

@crojack
Copy link
Author

crojack commented Dec 20, 2024

dimming of the area outside of the rectangular selection

When using such tool, it's often useful to highlight several regions - meaning, to dim everything except several regions.

If you apply it one region at a time, the result would be different

I think we should just add a dimming slider to our widget toolbar. It's much much easier than adding a separate tool.

@Photon89
Copy link
Member

Nice! Does the dimming work with rectangles only or with all kind of shapes?

If with rectangles only, I'd propose to make a new tool/button for it. Then only show the dimming slider which this tool is active.

@Photon89
Copy link
Member

I need to pack all the shapes (or all polygons) into one chooser to make room for pixelize, censor, crop, and any new feature we might add in the future.

That's a good idea! What about the way Xournal++ solved this: A button with a dropdown arrow next to it. When clicking the button, the curretly active shape is activated. In the dropdown menu the shape can be changed. When changed, it also alters the tool's icon. Another click on the button disables the shape tool (and reverts to the freehand tool). I need to add that the shapes are treated as a modifier to the freehand tool there, though, not as distinct tools.

Auswahl_708 Auswahl_709 Auswahl_710

I've never got your response about toggling the drawing toolbar from the top (under the main toolbar) to the left (what is what you preffer) and vice-versa, as well an option to hide the main toolbar?

I like the way you implemented it very much! Sorry for not pointing that out earlier!

@DarthGandalf
Copy link
Member

From the pictures you posted, it looks nice, but we can give more meaningful feedback once we're able to touch the new UI ourselves

@crojack
Copy link
Author

crojack commented Dec 20, 2024

What about the way Xournal++ solved this:

I didn't know Xournal++ existed. Wow. I think that you've just killed my motivation to continue with this application.

@Photon89
Copy link
Member

Photon89 commented Dec 22, 2024

Do you mean the screen resolution or the image resolution? I think, they should look the same on a given screen, independent of the image size and the zoom level. But they should probably have more pixels on a high resolution screen to be equally well visible.

Another minor thing that I stumbled upon: When saving a file, it should propose the filename of the currently edited file (possibly with some suffix or prefix to prevent accidental overwriting). Also the confirmation message after saving is rather unusual.

Some apps show an asterisk in the window name when a file has unsaved changes, which disappears after saving. Don't know how hard this is to implement, but I think, that would be more convenient than a confirmation message on saving.

@DarthGandalf
Copy link
Member

For some reason the video in latest post doesn't open for me.

But I think that size of handles shouldn't change with zoom, but should be the same number of pixels on screen, regardless of current zoom of the picture. Basically, agreeing with @Photon89

But I have a much better idea. And you'll love it.

Sorry, I don't. When one ever needs to change the handle's size? Only if it's wrong. If you really want it to be configurable, just put it to settings. But if it's a good size from the start (and doesn't change when you zoom) there's no need for that either.

I've noticed that every manual drag event creates many items in the undo list, which makes it painful to actually undo anything. I needed to hold Ctrl-Z for many seconds watching how my triangles are changing size in reverse :) And while doing that, I see a shadow of the original shape together with the one being unresized. In fact, if I unpress Ctrl-Z while the shadow is visible, I now get both these shapes, and can resize them separately!

In the text tool, have you tested it only with ASCII characters? I can't type anything in Cyrillic.

What's the point of triangle, tetragon and pentagon? It feels like you implemented an arbitrary polygon (I haven't checked, maybe you didn't), but for some reason decided only to make it possible to do these numbers of corners

Another issue: if the image doesn't fit the window, and scrollbar is visible, and it's actually scrolled, the mouse coordinates are off.

@DarthGandalf
Copy link
Member

I meant image resolution, but of course it's both.

Screen resolution shouldn't matter for this at all, only its DPI. Handles on small image should be the same as handles on a large image, so image resolution also shouldn't matter

@Photon89
Copy link
Member

That's a good point, it should be possible to calculate a good handle size by multiplying some suiting value with the DPI value.

@DarthGandalf
Copy link
Member

I wrote earlier that

Ah, missed it, sorry

No need to snap at me and be so harsh.

I didn't try to. Sorry if I sound too hard, my intention was to test and report bugs to you.

if you could tell me how to increase the size of selection handles in Shutter's Drawing Tool when high resolution images are loaded?

It's easily possible that the old drawing tool doesn't do the right thing either.

@crojack
Copy link
Author

crojack commented Dec 22, 2024

That's a good point, it should be possible to calculate a good handle size by multiplying some suiting value with the DPI value.

Yeah, it's possible, but I don't have a clue how to do that. Until you guys "calculate a good handle size by multiplying some suiting value with the DPI value" I'll be using handle size slider. I love it.

@DarthGandalf
Copy link
Member

looking at the code, please don't do this $ENV{DISPLAY} = ':0';. There are valid reasons to use DISPLAY other than :0, and either way, this variable should be set already by the system, or overridden explicitly by the user.
Why do you need $ENV{GDK_BACKEND} = 'wayland'; - GDK doesn't select the right backend itself if you run on wayland?

@Photon89
Copy link
Member

Do we do anything related to DPI in Shutter itself, actually? If so, maybe this can be adapted to the drawing tool? Something like this:

$self->{_dpi_scale} = Gtk3::Window->new('toplevel')->get('scale-factor');

@Photon89
Copy link
Member

This should stay, right?

The $x11_supported stuff should be irrelevant for the Image Annotator, I think. It is used in Shutter to decide how to do the screen capturing.

@DarthGandalf
Copy link
Member

This should stay, right?

Unless you use Wnck library, you won't need any of that. Shutter uses Wnck to enumerate list of windows to take screenshot of a single window.

I'm NOT a Perl programmer

Just to clarify: I'm not blaming you, not attacking, etc. My comments were to raise your awareness about certain issues with the product or the code which I've noticed, so that these issues can be fixed, and therefore product improved (it's hard to fix something if you don't know about the issue, right?). I understand this is alpha version, and not expecting everything to work perfectly yet.

While I "know" perl, I can't say I'm a good perl programmer. Seeing you writing this annotator tool, I think you're not any worse perl programmer than me.

Shutter codebase is not very good codebase either, I wouldn't recommend using it to learn how to write Perl code :\ That's one of reasons why I started the Rust port a while ago, though never got after proof of concept stage yet.

@DarthGandalf
Copy link
Member

By "any of that" I meant wnck_screen etc. Probably $window->set_title('Image Annotator'); is still necessary

@crojack
Copy link
Author

crojack commented Dec 23, 2024

By "any of that" I meant wnck_screen etc. Probably $window->set_title('Image Annotator'); is still necessary

😂

@Photon89
Copy link
Member

Nice!

I think, it is customary to add the extension to the proposed file name. Like "annotation.png" instead of just "annotation". Also, what about including the original background image's name? For example, of I open "image.png" for annotation, then it could propose "image_annotated.png" as default save name.

I noticed that zooming with Ctrl + scroll doesn't work, but possibly I am using outdated an outdated code snapshot.

Regarding the handle size, I experimented with Gtk3::Window->new('toplevel')->get('scale-factor') and it turned out that this is just an integer scale factor for HiDPI screens. But thinking about it, the handles are part of the GUI, so they shouldn't scale any different. If somebody is using a high resolution screen such that all GUI elements like buttons, icons, menus etc. look small on the screen, then I think that the handles, as part of the GUI, should be small as well. And also keep that fixed size when zooming in and out, because they aren't part of the image which is being zoomed.

@DarthGandalf
Copy link
Member

And also keep that fixed size when zooming in and out, because they aren't part of the image which is being zoomed.

From what I understand, zooming as part of the image is how it's currently implemented

@Photon89
Copy link
Member

Yes, what I meant is, how it looks and feels for the user, at least in similar apps that I know of.

@Photon89
Copy link
Member

Oh, I guess, I used an outdated code snapshot where the handle size scaled with the image when zooming the image. Sorry for the confusion! So the handles don't change their size when zooming in and out?

I was wondering whether you intend to create a project/repository for Image Annotator, so it is easier to grab the most recent version?

@DarthGandalf
Copy link
Member

DarthGandalf commented Dec 30, 2024

It's "3. Something else"

In both of these examples the handles have the same size of 10 pixels:

You made it 10 pixels of image. They should be 10 (or however many) pixels of screen.

The primitive itself of course should scale together with the image zoomed. But the handles size should stay constant on screen, independent from the image zoom level.

In the screen space handles should stay the same size always. But if you insist on using image space (apparently, you do, not sure why), then the relative size of handles should change with zoom, so that they stay the same in screen space.

@crojack
Copy link
Author

crojack commented Dec 30, 2024

The primitive itself of course should scale together with the image zoomed. But the handles size should stay constant on screen, independent from the image zoom level. In the screen space handles should stay the same size always.

Oh, you mean like in Ksnip? Oh, yeah, sure. Now I understand. I didn't understand before because the Shutter Drawing Tool handles do not stay the same independent from the image zoom level. They scale/resize together with the image and the primitive they are attached to. But, no problem, now I know what you guys mean. Why is that not implemented in Shutter's DrawingTool?

@DarthGandalf
Copy link
Member

apparently, you do, not sure why

I guess that's because you are drawing both the primitive and its handles on the image itself, allowing that to be zoomed together with the image. I see 2 solutions:

  1. draw the primitive on the image, but handles on the screen (need to calculate the correct locations of the handles)
  2. reverse zoom the handles before drawing them on the image

@crojack
Copy link
Author

crojack commented Dec 30, 2024

You haven't seen my answer :)

@DarthGandalf
Copy link
Member

Oh, you mean like in Ksnip?

I don't know what it does. Possibly?

the Shutter Drawing Tool handles do not stay [...] Why is that not implemented in Shutter's DrawingTool?

yeah, again, I'm not saying that the old tool is very good. And I don't know whether the wrong behavior was there from the start, or when I was porting Shutter to Gtk3

I'm still surprised you decided to reuse some part of the old codebase

@crojack
Copy link
Author

crojack commented Dec 30, 2024

I don't know what it does. Possibly?

It does exactly what you want. The handles stay always the same, they have the same size, independent of image zooming.

@Photon89
Copy link
Member

I didn't understand before because the Shutter Drawing Tool handles do not stay the same independent from the image zoom level. They scale/resize together with the image and the primitive they are attached to.

Indeed, I never noticed that!

Oh, you mean like in Ksnip?

Xournal++ also does it this way.

@crojack
Copy link
Author

crojack commented Dec 30, 2024

Handle size slider still works and if you increase or decrease the size of the handles their new size will still stay same on screen regardless of zooming :)

@Photon89
Copy link
Member

Nice! The screenshots look good!

@DarthGandalf
Copy link
Member

Implemented fixed on-screen-size selection handles

Cool!

Handle size slider still works and if you increase or decrease the size of the handles their new size will still stay same on screen regardless of zooming :)

Is it still necessary though? :P If it is, I suggest hiding it in settings to preserve some space on the toolbar, since changing the size isn't needed very often

@DarthGandalf
Copy link
Member

Also I suggest trying HiDPI mode to decide whether the size of the handle needs to be multiplied by the current dpi scale or not

@DarthGandalf
Copy link
Member

if there is enough space on the toolbars and in the menus.

Ah ok, I thought you said there's not enough space.

@DarthGandalf
Copy link
Member

Well, you need to consider people with other screen sizes too, not just yours

@DarthGandalf
Copy link
Member

So yes, I am thinking about other users.

Great!

What makes you think I don't?

Just this phrase itself: "I thought it wasn't because I was running hiDPI. Now when I run native 4k thre is a lot of space in the toolbars."

Because if someone else runs hidpi and not native 4k screen, will they not have much space in the toolbars again?

1280x720 screen resolution uses 24x24 icons, 1920x1080 = 32x32 icons

Ah, interesting

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Question about installing, configuring or using Shutter
Projects
None yet
Development

No branches or pull requests

3 participants