Rewrite the algorithm that sets smallIcon/largeIcon in Decorations class #1879
Conversation
Test Results 509 files ±0 509 suites ±0 10m 20s ⏱️ ±0s For more details on these failures, see this check. Results for commit 0892529. ± Comparison against base commit 7f381e8. ♻️ This comment has been updated with latest results. |
fedejeanne
force-pushed
the
fix_blurry_shell_icon
branch
from
172580b to
2089d41
Compare
|
Failing test is unrelated: #1843 |
bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/Decorations.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Simplifying this functionality and improve comprehensibility is a good thing.
I left some proposal for improving the code and making the logic comprehensible. Most essential for me would be to assign the compares values that are hard to understand (system metrics, image data at specific zooms) to comprehensive, self-explaining variables, as that's exactly the part that confused me all the time and made it difficult to understand what happens here.
Apart from that, formatting does currently seems to be broken at several places (in particular, missing whitespaces).
bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/Decorations.java
Outdated
Show resolved
Hide resolved
bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/Decorations.java
Outdated
Show resolved
Hide resolved
bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/Decorations.java
Outdated
Show resolved
Hide resolved
bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/Decorations.java
Outdated
Show resolved
Hide resolved
bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/Decorations.java
Outdated
Show resolved
Hide resolved
bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/Decorations.java
Outdated
Show resolved
Hide resolved
fedejeanne
force-pushed
the
fix_blurry_shell_icon
branch
3 times, most recently
from
b4659b3 to
74f094a
Compare
Don't sort, compare and keep the image that is closest to the desired dimensions, pixel transparency and pixel depth. The new method "isCloserThan" is based on the (now deleted) "compare" method and returns true only if "compare" would have returned -1 Reduce visibility of Decorations::setImages
fedejeanne
force-pushed
the
fix_blurry_shell_icon
branch
from
99093d5 to
0892529
Compare
|
Perfect! 0892529 only changes the formatting of the affected code. I used |
|
Test failure is unrelated (test timeout) since the last change only changed formatting. Merging. |
The output is the same but the new algorithm doesn't sort and select the 1st one, it directly searches for the best one.