eclipse.platform.swt icon indicating copy to clipboard operation
eclipse.platform.swt copied to clipboard

Follow-Up Feature: Icon-Enablement with Rasterization of SVGs

Open Michael5601 opened this issue 1 year ago • 5 comments

This draft outlines the follow-up functionality for the PR (Feature Proposal: Rasterization of SVGs at Runtime for Eclipse Icons).

The commit 4e6abc7c21d10d293a68288170ef91e086d6fef1 contains the new functionality that extends the base functionality of the mentioned PR. All future changes to the PR will be performed to this draft.

The new functionality extends the current automatic customization of icons in the Image(Device device, Image srcImage, int flag) constructor. While all of the core functionality is implemented within SWT, some changes are required in Platform UI, which can be found in the following Draft.

When the Image constructor is invoked, the new functionality attempts to create a graphically customized icon by passing a specific flag (e.g., SWT.IMAGE_DISABLE) to the rasterization functionality introduced in the base PR. This functionality is enhanced by a preprocessing step that automatically adds a filter to the SVG before rasterization. These filters are designed to ensure that the resulting icons resemble the current pre-created disabled/gray icons loaded at runtime.

It is important to note that the automatic icon customization differs between GTK and Cocoa/Win32. As such, the icons generated with the new functionality may not align perfectly with the automatically customized GTK icons. However, this discrepancy is also present with the current pre-created icons, so the impact is minimal. Additionally, most icons are pre-created and not automatically customized at runtime.

Below is a comparison between the current pre-created icons and the icons customized automatically at runtime using the new functionality. The color/saturation/brightness can be changed by adjusting the filter:

Pre-created and scaled down to 125% device zoom: image

Automatically customized at runtime with the new functionality: image

The following tasks need to be completed for this draft:

  1. Decide whether the unification of the behavior across different operating systems should occur before merging the new functionality.
  2. Merge the main feature upon which this draft is based.
  3. Remove disabled icon paths from the plugin.xml files to ensure the automatic customization is triggered.
  4. Write documentation (JavaDocs) for the new API.
  5. Provide regression tests and a performance comparison between automatic customization and FileIO for pre-created icons. The possibility exists that the rasterization with pre-processing is faster than the additional FileIO to load the pre-created icons.

Fixes https://github.com/eclipse-platform/eclipse.platform.swt/issues/1438.

Michael5601 avatar Dec 11 '24 16:12 Michael5601

Below is a comparison between the current pre-created icons and the icons customized automatically at runtime using the new functionality. The color/saturation/brightness can be changed by adjusting the filter:

Pre-created and scaled down to 125% device zoom: image

Automatically customized at runtime with the new functionality: image

To be honest I almost cannot see a difference. So I think this should not be a problem.

BeckerWdf avatar Dec 12 '24 07:12 BeckerWdf

To be honest I almost cannot see a difference. So I think this should not be a problem.

This is a good thing for me. :) We could utilize the new behaviour from this draft, which means the pre-created PNGs can be removed and we have a little improvement in the look of the icons without changing their appearance (gray tone) too much.

Michael5601 avatar Dec 12 '24 10:12 Michael5601

Test Results

  218 files   -    321    218 suites   - 321   10m 47s ⏱️ - 19m 12s 4 330 tests ±     0  4 273 ✅  -     47  57 💤 +48  0 ❌  - 1  4 477 runs   - 12 102  4 420 ✅  - 12 057  57 💤  - 44  0 ❌  - 1 

Results for commit 4afe4996. ± Comparison against base commit 22e8829e.

This pull request removes 37 and adds 37 tests. Note that renamed tests count towards both.
AllGTKTests org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicASCII_dollarSign
AllGTKTests org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicASCII_emptyString
AllGTKTests org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicASCII_letterA
AllGTKTests org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicASCII_letters
AllGTKTests org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicUTF16LE_null
AllGTKTests org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicUTF16_AsciiLetters
AllGTKTests org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicUTF16_Asciiletter
AllGTKTests org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicUTF16_LotsOfLetters
AllGTKTests org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicUTF16_letter
AllGTKTests org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicUTF16_letters
…
AllWin32Tests org.eclipse.swt.graphics.ImageWin32Tests ‑ testImageDataForDifferentFractionalZoomsShouldBeDifferent
AllWin32Tests org.eclipse.swt.graphics.ImageWin32Tests ‑ testImageShouldHaveDimesionAsPerZoomLevel
AllWin32Tests org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testByteArrayTransfer
AllWin32Tests org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testFileTransfer
AllWin32Tests org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testHtmlTransfer
AllWin32Tests org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testImageTransfer_fromCopiedImage
AllWin32Tests org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testImageTransfer_fromImage
AllWin32Tests org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testImageTransfer_fromImageData
AllWin32Tests org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testImageTransfer_fromImageDataFromImage
AllWin32Tests org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testRtfTransfer
…
This pull request removes 2 skipped tests and adds 3 skipped tests. Note that renamed tests count towards both.
AllGTKTests org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_Heuristic_specialSingleCases
org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_Heuristic_specialSingleCases
AllWin32Tests org.eclipse.swt.tests.win32.Test_org_eclipse_swt_events_KeyEvent ‑ testEnglishUs_multipleLetters
AllWin32Tests org.eclipse.swt.tests.win32.Test_org_eclipse_swt_events_KeyEvent ‑ testEnglishUs_unorderedCtrlC
AllWin32Tests org.eclipse.swt.tests.win32.Test_org_eclipse_swt_events_KeyEvent ‑ testEnglishUs_unpairedKeyUp
This pull request skips 47 tests.
org.eclipse.swt.tests.junit.Test_org_eclipse_swt_browser_Browser ‑ test_BrowserFunction_callback[browser flags: 0]
org.eclipse.swt.tests.junit.Test_org_eclipse_swt_browser_Browser ‑ test_LocationListener_LocationListener_ordered_changing[browser flags: 0]
org.eclipse.swt.tests.junit.Test_org_eclipse_swt_browser_Browser ‑ test_LocationListener_evaluateInCallback[browser flags: 0]
org.eclipse.swt.tests.junit.Test_org_eclipse_swt_browser_Browser ‑ test_OpenWindowListener_evaluateInCallback[browser flags: 0]
org.eclipse.swt.tests.junit.Test_org_eclipse_swt_browser_Browser ‑ test_StatusTextListener_hoverMouseOverLink[browser flags: 0]
org.eclipse.swt.tests.junit.Test_org_eclipse_swt_browser_Browser ‑ test_setForegroundAlphaLorg_eclipse_swt_graphics_Color[browser flags: 0]
org.eclipse.swt.tests.junit.Test_org_eclipse_swt_custom_CCombo ‑ test_setForegroundAlphaLorg_eclipse_swt_graphics_Color
org.eclipse.swt.tests.junit.Test_org_eclipse_swt_custom_CLabel ‑ test_setForegroundAlphaLorg_eclipse_swt_graphics_Color
org.eclipse.swt.tests.junit.Test_org_eclipse_swt_custom_CTabFolder ‑ test_setForegroundAlphaLorg_eclipse_swt_graphics_Color
org.eclipse.swt.tests.junit.Test_org_eclipse_swt_custom_StyledText ‑ test_setForegroundAlphaLorg_eclipse_swt_graphics_Color
…

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar Feb 24 '25 08:02 github-actions[bot]

What is the status of this one?

akurtakov avatar Apr 08 '25 18:04 akurtakov

What is the status of this one?

We are currently working on this PR in Platform UI, so that SVGs can be also used by external consumers like URLImageDescriptor or FileImageDescriptor in Jface. With this also Composite Icons can be loaded by using SVGs.

The icon disablement as proposed in this Draft needs further adjustments as it does not work in Cocoa yet and also new API needs to be added so that icons can be created with arbitrary Hue, Saturation and Brightness at runtime. Additionally we should first merge this PR to unify the disablement algorithms for all OS.

Apart from that the functionality in this draft is working and allows the disablement for all icons by perprocessing SVGs before rasterization. Please note that due to the different implementation for cocoa it could happen that the changes proposed in this draft will be dismissed completely.

Michael5601 avatar Apr 09 '25 08:04 Michael5601