xiaoyifang / goldendict-ng

The Next Generation GoldenDict
https://xiaoyifang.github.io/goldendict-ng/
Other
1.7k stars 95 forks source link

feature: High-quality dictionary icons in toolbar #1750

Closed KonstantinDjairo closed 2 months ago

KonstantinDjairo commented 2 months ago

some of the code added to prevent crashes had a bad side effect, causing the icons to get a bad quality. two line of codes in two different files were causing that downscaling, the first line was removed, the second had its value changed to a higher value, both conditions were tested and the result is pretty good:

2024-09-08_23-06

shenlebantongying commented 2 months ago

Surprisingly, setDictionaryIconSize cannot be removed because Show Small Icons in Toolbars needs this.

Change initial value will cause problems to Show Small Icons in Toolbars.

Highly related: https://github.com/xiaoyifang/goldendict-ng/issues/1459

shenlebantongying commented 2 months ago

https://github.com/xiaoyifang/goldendict-ng/issues/1459 need another day to do.

However, I pushed a change to enable a hidden feature that enables the user to set dictionaryBar icon size in qt style sheet.

MainWindow #dictionaryBar {
    icon-size: 64px;
}

Note that it will be bound by 64.

Edit: this doesn't work for actually implemented version.

KonstantinDjairo commented 2 months ago

1459 need another day to do.

However, I pushed a change to enable a hidden feature that enables the user to set dictionaryBar icon size in qt style sheet.

MainWindow #dictionaryBar {
  icon-size: 64px;
}

Note that it will be bound by 64.

can that have an option in the preferences?

shenlebantongying commented 2 months ago

can that have an option in the preferences?

I don't know yet. Need to find free time to implement it first 😅

Can you please double-check my code? Then let's merge this first, and consider an option for larger icons later.

KonstantinDjairo commented 2 months ago

can that have an option in the preferences?

I don't know yet. Need to find free time to implement it first 😅

Can you please double-check my code? Then let's merge this first, and consider an option for larger icons later.

alright, i'll test on my guix and check the results

KonstantinDjairo commented 2 months ago

These are the results

2024-09-09_12-06 2024-09-09_12-04_1 2024-09-09_12-04

one thing that you can notice is that somehow the icons are too far from each other, it's not a square anymore... another thing that can be noticed in the bigger screenshot is that even tho the quality is better than the one in the main branch, you can notice some pixel-ish effect in it, so i bet it's having a downscale , right?

shenlebantongying commented 2 months ago

I think the rectangle icon and pixels effect is caused by none-square initial toolbar icon size?

I pushed a fix, does it work?

KonstantinDjairo commented 2 months ago

I think the rectangle icon and pixels effect is caused by none-square initial toolbar icon size?

I pushed a fix, does it work?

there's a bit of blur image

if you compare to my first image

image

maybe when choosing the icon size, you can compare the height and width numerically and pick the other that's higher, this way it will avoid downscale, but it can also cause upscaling, but i dont think it's a problem

sonarcloud[bot] commented 2 months ago

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

shenlebantongying commented 2 months ago

Ok, let's merge this :)

KonstantinDjairo commented 2 months ago

Ok, let's merge this :)

still blurry but i think it'd require more than one PR to solve this

shenlebantongying commented 2 months ago

Could the reason be high resolution screen?

For a 4x density monitor and the toolbar icon's logical size 21, then it would require 21*4=84?

KonstantinDjairo commented 2 months ago

I think the rectangle icon and pixels effect is caused by none-square initial toolbar icon size? I pushed a fix, does it work?

there's a bit of blur image

if you compare to my first image

image

maybe when choosing the icon size, you can compare the height and width numerically and pick the other that's higher, this way it will avoid downscale, but it can also cause upscaling, but i dont think it's a problem

for example, if you look at this comparison, both are in the same monitor. and indeed your solution of selecting the "height" value was good, but somehow it made the icons a bit blurry

for some reason when i first removed the value of 21 as we did in the beginning, and set a high value in another variable: https://github.com/xiaoyifang/goldendict-ng/pull/1750/commits/11314d690630382fe2ccc5700ee7d1cb21d0980e the result was the one in the picture, it's pretty sharp and there's no blur

somehow the barsize is downscaling the icons? i dont really know

KonstantinDjairo commented 2 months ago

here's the comparison between the original icon (on the right) and the one produced by goldendict-ng. the one on the left is blurry image