Skip to content

Remove duplicate representation of alpha values in image data from SVG #2254

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

HeikoKlare
Copy link
Contributor

@HeikoKlare HeikoKlare commented Jun 19, 2025

The image data generated for a rasterized SVG currently uses a 32 bit data array and an alpha values array. The alpha values for every pixel are stored in both the alpha values array as well as the alpha channel of the 32 bit data array. This is a redundant representation not giving any benefits but just leading to unnecessary memory consumption.

This change adapts the SVG rasterization to create image data with a 24 bit data array, such that the alpha values are only stored in the alpha array.

This was detected via and would also resolve:

I have to admit that I am not completely sure when a 32 bit data array can/must be used together with an alpha data array, thus having duplicate representation of alpha data. When searching in platform code, I see several places creating a 24bit data array and an additional alpha array, but I see about the same number of places that create a 32bit data array and an additional alpha array. I guess there is some difference depending on whether the RGB values encode the alpha values as well (I have seen something like in disabled image generation on Linux/GTK), but if that's not the case I am not sure whether or when one or the other option is useful or even necessary.
If possible, I would be in favor of having a redundancy-free representation of data, which is why I think the proposed change is reasonable. Even more considering that it will fix the mentioned issue.

So far, I have tested this change on macOS and Windows and did not find any issues. It needs to be tested on Linux as well to ensure that there is no different in how the Linux implementation of Image treats such data.
I've also tested the proposal on Linux and did not find any issues with the loaded SVGs.
I would assume that whether this works or not does not depend on the usage context but as long as we see that the alpha value handling of the loaded image data works in general, it will work everywhere since after loading the image data it is usually transformed into an image with an according OS handle to which the data is transformed.

Copy link
Contributor

github-actions bot commented Jun 19, 2025

Test Results

   545 files  + 6     545 suites  +6   35m 18s ⏱️ + 8m 1s
 4 402 tests +37   4 384 ✅ +35   18 💤 +3  0 ❌  - 1 
16 732 runs  +37  16 592 ✅ +35  140 💤 +3  0 ❌  - 1 

Results for commit 9c99cca. ± Comparison against base commit b339485.

♻️ This comment has been updated with latest results.

@HeikoKlare HeikoKlare force-pushed the svg-rasterizer-24bitdata branch from 7473374 to 67ce839 Compare June 19, 2025 12:03
The image data generated for a rasterized SVG currently uses a 32 bit
data array and an alpha values array. The alpha values for every pixel
are stored in both the alpha values array as well as the alpha channel
of the 32 bit data array. This is a redundant representation not giving
any benefits but just leading to unnecessary memory consumption.

This change adapts the SVG rasterization to create image data with a 24
bit data array, such that the alpha values are only stored in the alpha
array.
@HeikoKlare HeikoKlare force-pushed the svg-rasterizer-24bitdata branch from 67ce839 to 04918a4 Compare June 19, 2025 14:19
@eclipse-platform-bot
Copy link
Contributor

This pull request changes some projects for the first time in this development cycle.
Therefore the following files need a version increment:

bundles/org.eclipse.swt.svg/META-INF/MANIFEST.MF

⚠️ 🚧 This PR cannot be modified by maintainers because edits are disabled or it is created from an organization repository. To obtain the required changes apply the git patch manually as additional commit.

Git patch
From bf1b813f6fe7ff79f704ad8ba8050f583814844c Mon Sep 17 00:00:00 2001
From: Eclipse Platform Bot <[email protected]>
Date: Thu, 19 Jun 2025 14:23:38 +0000
Subject: [PATCH] Version bump(s) for 4.37 stream


diff --git a/bundles/org.eclipse.swt.svg/META-INF/MANIFEST.MF b/bundles/org.eclipse.swt.svg/META-INF/MANIFEST.MF
index dc70fca625..1ee3b0126d 100644
--- a/bundles/org.eclipse.swt.svg/META-INF/MANIFEST.MF
+++ b/bundles/org.eclipse.swt.svg/META-INF/MANIFEST.MF
@@ -1,7 +1,7 @@
 Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-SymbolicName: org.eclipse.swt.svg
-Bundle-Version: 3.130.0.qualifier
+Bundle-Version: 3.130.100.qualifier
 Automatic-Module-Name: org.eclipse.swt.svg
 Bundle-Name: %fragmentName
 Bundle-Vendor: %providerName
-- 
2.49.0

Further information are available in Common Build Issues - Missing version increments.

@HeikoKlare HeikoKlare marked this pull request as ready for review June 19, 2025 15:24
@HeikoKlare
Copy link
Contributor Author

@HannesWell since you were deeply involved in the development of the SVG feature, maybe you want to have a look at this?

@HannesWell
Copy link
Member

I have to admit that I am not completely sure when a 32 bit data array can/must be used together with an alpha data array, thus having duplicate representation of alpha data. When searching in platform code, I see several places creating a 24bit data array and an additional alpha array, but I see about the same number of places that create a 32bit data array and an additional alpha array. I guess there is some difference depending on whether the RGB values encode the alpha values as well (I have seen something like in disabled image generation on Linux/GTK), but if that's not the case I am not sure whether or when one or the other option is useful or even necessary.

Unfortunately I'm not familiar with these details either, but I wonder why there is a separated array for alpha values in the first place, if they can be represented in 32bit arrays?

@laeubi
Copy link
Contributor

laeubi commented Jun 19, 2025

It all depends on the color model. Even though RGBA is quite common one can have any other kind of encoding e.g. with 4bits/color (what would only need 2 bytes) and one alpha channel. But one can of course also use model with higher resolution (even floats).

Such an Alpha mask could even be 1 bit only that is common for old formats like GIF files (but of course gives not quite smooth results).

@laeubi
Copy link
Contributor

laeubi commented Jun 19, 2025

This is a redundant representation [...] but just leading to unnecessary memory consumption

By the way I think this statement is wrong, unless you used a color palette you won't save any memory as there is no (native) 24bit primitive data type in java.

Strike that.. SWT uses a byte array here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove duplicate representation of alpha values in image data from SVG
4 participants