#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: | ||
Platform: | All |
Description (last modified by )
Attachments (5)
Change History (29)
by , 8 years ago
Attachment: | Oversized.png added |
---|
comment:1 by , 8 years ago
patch: | 0 → 1 |
---|
comment:2 by , 8 years ago
Description: | modified (diff) |
---|
comment:3 by , 8 years ago
patch: | 1 → 0 |
---|
comment:4 by , 8 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:6 by , 6 years 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 , 6 years 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 , 6 years 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 , 6 years 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 large 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.
comment:10 by , 6 years ago
Got it. I'll adjust my patch to draw 32x32 or 16x16 based on the featured package setting
comment:11 by , 6 years 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
by , 6 years ago
Attachment: | larger-icons.png added |
---|
comment:13 by , 6 years 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 , 6 years ago
Attachment: | HaikuDepotFeaturedPkgs.png added |
---|
comment:14 by , 6 years 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 , 6 years 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 , 6 years ago
Thanks David; ah sorry - you are right. I had the initial patch applied still. I'll try again soon.
comment:17 by , 6 years 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.
by , 6 years ago
comment:19 by , 6 years ago
patch: | 0 → 1 |
---|
by , 6 years ago
comment:20 by , 6 years ago
comment:21 by , 6 years 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 , 6 years 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 , 6 years 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
comment:24 by , 6 years ago
Fix for the featured package list icon drawing issue mentioned by diver is at: https://review.haiku-os.org/#/c/haiku/+/443
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.