Opened 3 years ago

Closed 16 months ago

Last modified 16 months ago

#13399 closed bug (fixed)

HaikuDepot allows oversized icons

Reported by: kallisti5 Owned by: apl-haiku
Priority: normal Milestone: Unscheduled
Component: Applications/HaikuDepot Version: R1/Development
Keywords: Cc:
Blocked By: Blocking:
Has a Patch: yes Platform: All

Description (last modified by kallisti5)

HaikuDepot seems to show oversized icons:

Attachments (5)

Oversized.png (54.1 KB ) - added by kallisti5 3 years ago.
larger-icons.png (7.1 KB ) - added by apl-haiku 16 months ago.
HaikuDepotFeaturedPkgs.png (95.7 KB ) - added by closequarters 16 months ago.
hd.png (154.3 KB ) - added by diver 16 months ago.
hd2.png (123.5 KB ) - added by diver 16 months ago.

Download all attachments as: .zip

Change History (29)

by kallisti5, 3 years ago

Attachment: Oversized.png added

comment:1 by kallisti5, 3 years ago

Has a Patch: set

comment:2 by kallisti5, 3 years ago

Description: modified (diff)

comment:3 by kallisti5, 3 years ago

Has a Patch: unset

comment:4 by diver, 3 years ago

Owner: changed from stippi to apl-haiku
Status: newassigned

comment:5 by apl-haiku, 3 years ago

It seems that the icons that are stored server side are 16x16, 32x32, 64x64 and are available in the downloaded tar-ball from the server. The icons are not filling the top and bottom of the otherwise transparent PNG so it looks like they are non-square, but they are actually square.

The desktop application is taking the 64x64 in preference and then not scaling it. Ideally the desktop application would take the most sensible sizing for the situation where it needs to render for. This is probably the change that needs to happen here in in LocalIconStore.cpp and then subordinate clients of this service.

The desktop application should also render the icon scaled if necessary.

Last edited 3 years ago by apl-haiku (previous) (diff)

comment:6 by closequarters, 16 months ago

So what is the actual desired result here in the package list view? The "normal size" icons are drawn 16x16. The icons causing the problems are 32x32 and 64x64.

I don't see a setting in HaikuDepot that controls which icon size that should be used. There also doesn't seem to be an "expand/collapse row" function that would take a 16x16 icon and scale it up under certain circumstances. Given the absence of these features, it seems the simplest solution is to draw every bitmap as a 16x16 bitmap, regardless of the original source resolution.

Is that indeed the desired behavior here (resizing to 16x16 so all rows appear the same)? Or are we looking for something more complex - e.g. change the row size to accommodate the full bitmap size?

comment:7 by apl-haiku, 16 months ago

I think a good first step would be to render for this purpose at 16x16. HaikuDepot may also have the icon available (downloaded from the server) at different sizes so it may be good to choose the right icon size for the job if it is available or otherwise render with scaling.

comment:8 by closequarters, 16 months ago

My solution for rendering the icons at 16x16 is here: https://review.haiku-os.org/c/haiku/+/347

I'm still somewhat confused when you say "choose the right icon size" given that the package list rows seem sized to accommodate at most a 16x16 icon. Other areas of the application do show the icon at a different scale already. (For example if you click on an icon with a 32x32 or 64x64 source bitmap, the "details" pane of HaikuDepot will show the icon at its actual size).

In any case, this fix should at least stop HaikuDepot from looking quite odd while scrolling through.

comment:9 by apl-haiku, 16 months ago

The HaikuDepotServer system allows the user to store a combination of;

  • HVIF
  • PNG (64x64)
  • PNG (32x32)
  • PNG (16x16)

When HaikuDepot does a refresh of meta-data from HaikuDepotServer over a network connection it will download a .tgz of the icons and will unpack them on the client. The icons for each package are then available to be used by the application.

Back in HaikuDepot in the list view the view is either showing large or small icons depending on the "Show only featured packages" switch. If the icons are shown large then it would be good if HaikuDepot would choose the larger PNGs that came from the server system. If the icons are shown small then it would be good if HaikuDepot would choose the smaller PNGs.

The rationale is that the icons may be mastered by a person at 16x16 for better viewing compared to the icons supplied at 32x32. Such that if you take the 32x32 icon and render it at 16x16 you might get a worse looking outcome than had you taken the correctly sized icon to start with.

Last edited 16 months ago by apl-haiku (previous) (diff)

comment:10 by closequarters, 16 months ago

Got it. I'll adjust my patch to draw 32x32 or 16x16 based on the featured package setting

comment:11 by closequarters, 16 months ago

Ok, after studying the code in more depth, I finally get all the points you were making, apl-haiku. So your desired change here would be that we have the ability to store a reference to each bitmap size that a particular package has. And then in each part of HaikuDepot, we would retrieve the correct reference based on the size we want to display. For example in the package list view we could retrieve the 16x16 version of the icon if it's available, since the package list basically only accomodates that bitmap size.

The current code simply gets the biggest/highest quality version of the icon and returns that in the PackageInfo::Icon(). So it doesn't care if there are low-res versions of a bitmap or not. It just uses the best icon version wherever an icon is needed. The actual source of the bug here isn't related to the bitmap size though, it's occurring because the views that render the icon are not enforcing a particular size of bitmap when drawing. If we want the display to be uniform, we need to draw the icons, regardless of their source size, to fit the particular area of the layout. I don't think autolayou can handle that.

I agree that drawing the correct size icon rather than scaling it to the size demanded by a particular view makes more sense. However, it would also involve a fair amount of refactoring of LocalIconStore, ServerIconExportUpdateProcess, and PackageInfo. I'm happy to do this work in a follow-up, but I think my current patch is a material improvement over the current code since it fixes the rendering of icons in package list and featured package list with very minimal code changes

So I'd like to propose that we consider my current patch as-is, since it will make HaikuDepot much more uniform and it already uses the highest quality icon available for scaling. https://review.haiku-os.org/c/haiku/+/347

comment:12 by apl-haiku, 16 months ago

This sounds very sensible; in any case it will be helpful.

by apl-haiku, 16 months ago

Attachment: larger-icons.png added

comment:13 by apl-haiku, 16 months ago

Hello David; The changes to PackageListView.cpp look to be working, but the change made to FeaturedPackagesView.cpp looks to be continuing to render 32x32 PNG icons at the smaller size. Was it your intention to fix this?

In the image above you can see the icons for "BeLife" and "BeMines" are rendered smaller than the others. The icons for these two applications are only available as 16x16 and 32x32 bitmaps.

by closequarters, 16 months ago

Attachment: HaikuDepotFeaturedPkgs.png added

comment:14 by closequarters, 16 months ago

apl-haiku, are you sure that you are using the latest patch I posted? Patch version 1 had changes only to PackageListView.cpp, while Patch version 2 had changes there as well as FeaturedPackagesView.cpp . Attached screenshot shows how it looks for me - in my test HaikuDepot the icons appear to be scaled up to 64x64

comment:15 by closequarters, 16 months ago

I got more time to myself today than I thought, so I went ahead and built a new patch that incorporates all suggested changes - namely that we now properly store all icon sizes that are provided by a particular package, and in situations where it is required, HaikuDepot requests a specific icon size and gets it, instead of needing to scale it.

https://review.haiku-os.org/c/haiku/+/353

I will say that this is a lot more code than my previous patch, and the only material improvement out of the box is in the package detail view (that is shown when you click on a package in the list), we now properly display a 32x32 icon - whereas before I think it just showed the best available icon size.

Having the Icon package sizes properly accounted for is kind of nice, it's good to know you can get the one you want, but I'm not sure the extra code is actually worth it. Either of these patches should do the trick though in terms of improving the display of HaikuDepot. I defer to the rest of the team as to which patch is actually better.

comment:16 by apl-haiku, 16 months ago

Thanks David; ah sorry - you are right. I had the initial patch applied still. I'll try again soon.

comment:17 by closequarters, 16 months ago

apl-haiku, per your preference expressed in Gerrit, I've abandoned the 353 more complex patch, leaving just the 347 patch. I also confirmed that my original value of 16 was correct through testing. If a value of 15 is used, the first column of the bitmap is cutoff when drawn.

I think we are ready for final review and merge.

comment:18 by waddlesplash, 16 months ago

Resolution: fixed
Status: assignedclosed

Merged

by diver, 16 months ago

Attachment: hd.png added

comment:19 by diver, 16 months ago

Has a Patch: set

by diver, 16 months ago

Attachment: hd2.png added

comment:20 by diver, 16 months ago

Now it looks like drawing mode is wrong somehow:

Also bottom image bitmap is twice as large as vector icons:

comment:21 by closequarters, 16 months ago

diver, could you explain more what you are seeing in the first screenshot about the drawing mode? I'm not seeing an obvious problem in that screenshot.

With regards to the "Package Info View", my patch did not address the rendering of that icon. It might make more sense to address that in a new ticket - maybe apl-haiku could provide the requirements - is the goal there to draw a fixed icon size (say scale to 64x64)?

comment:22 by diver, 16 months ago

The icons in the first screenshot now have dirty black background around them. It wasn't like that before the patch.

comment:23 by closequarters, 16 months ago

diver - thanks for clarifying. I have a fix for the drawing on the featured packages view. I'll be submitting it in the near future along with a fix for #11675. All I had to do was change the drawing mode from B_OP_OVER to B_OP_ALPHA

Last edited 16 months ago by closequarters (previous) (diff)

comment:24 by closequarters, 16 months ago

Fix for the featured package list icon drawing issue mentioned by diver is at: https://review.haiku-os.org/#/c/haiku/+/443

Note: See TracTickets for help on using tickets.