Skip to content

Conversation

@laeubi
Copy link
Contributor

@laeubi laeubi commented Oct 22, 2025

Currently we have the problem that I want to replace some icons with a SVG variant. Currently Icons can be organized in folders like /icons/16x16/search.png, /icons/32x32/search.png and so on for high resolution support. Currently if one want to replace such structure with SVG it is required to scale down the SVG to 16x16 pixel document size as otherwise they get rendered at there native size (what usually is much larger).

As it is not really desirable to restrict the size of the SVG design for technical reasons, JFace now can detect two cases:

  1. the SVG is places in a folder with "classic" folder layout the size is extracted and passed down as a hint for dynamic sizable icons 2) one can additionally add a query parameter, e.g. if I have an icon like /icons/obj16/search.svg and I have two places where I want to use it one for a toolbar (16x16) and once for a Wizard Images (usually 128x128) I can use the url
    bundle:/example.id/icons/obj16/search.svg?size=16x16 and bundle:/example.id/icons/obj16/search.svg?size=128x128 to accomplish this task without the need to even store two SVGs

@github-actions
Copy link
Contributor

github-actions bot commented Oct 22, 2025

Test Results

 1 998 files   -   899   1 998 suites   - 899   44m 28s ⏱️ - 1h 40m 49s
 6 512 tests  - 1 724   6 465 ✅  - 1 523   41 💤  - 207   6 ❌ + 6 
18 456 runs   - 4 396  18 271 ✅  - 3 792  167 💤  - 622  18 ❌ +18 

For more details on these failures, see this check.

Results for commit f7e1298. ± Comparison against base commit 02fc196.

This pull request removes 1735 and adds 11 tests. Note that renamed tests count towards both.
UiTestSuite ActivitiesTestSuite ActivityPreferenceTest ‑ testActivityPreference
UiTestSuite ActivitiesTestSuite DynamicTest ‑ testActivityListener
UiTestSuite ActivitiesTestSuite DynamicTest ‑ testActivityManagerListener
UiTestSuite ActivitiesTestSuite DynamicTest ‑ testActivityPatternBindings
UiTestSuite ActivitiesTestSuite DynamicTest ‑ testCategoryListener
UiTestSuite ActivitiesTestSuite DynamicTest ‑ testDynamicRegistry
UiTestSuite ActivitiesTestSuite DynamicTest ‑ testEnabledActivities
UiTestSuite ActivitiesTestSuite DynamicTest ‑ testIdentifiersListener
UiTestSuite ActivitiesTestSuite DynamicTest ‑ testSizes
UiTestSuite ActivitiesTestSuite ImagesTest ‑ testActivityImages
…
AllTests AllImagesTests URLHintProviderTest ‑ testNoHintDefaultSize
AllTests AllImagesTests URLHintProviderTest ‑ testPathBasedHintDetection16x16
AllTests AllImagesTests URLHintProviderTest ‑ testPathBasedHintDetection32x32
AllTests AllImagesTests URLHintProviderTest ‑ testPathBasedHintDetectionZoom150
AllTests AllImagesTests URLHintProviderTest ‑ testPathBasedHintDetectionZoom200
AllTests AllImagesTests URLHintProviderTest ‑ testQueryParameterHintDetection
AllTests AllImagesTests URLHintProviderTest ‑ testQueryParameterHintDetection64x64
AllTests AllImagesTests URLHintProviderTest ‑ testQueryParameterPrecedenceOverPath
AllTests AllImagesTests URLHintProviderTest ‑ testQueryParameterRectangularSize
AllTests AllImagesTests URLHintProviderTest ‑ testQueryParameterWithMultipleParams
…

♻️ This comment has been updated with latest results.

Comment on lines 18 to 45
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import org.eclipse.swt.graphics.Point;

class URLHintProvider implements Supplier<Point> {

private static final Pattern QUERY_PATTERN = Pattern.compile("&size=(\\d+)x(\\d+)"); //$NON-NLS-1$
private static final Pattern PATH_PATTERN = Pattern.compile("/(\\d+)x(\\d+)/"); //$NON-NLS-1$

private URL url;

public URLHintProvider(URL url) {
this.url = url;
}

@Override
public Point get() {
String query = url.getQuery();
Matcher matcher;
if (query != null && !query.isEmpty()) {
matcher = QUERY_PATTERN.matcher("&" + query); //$NON-NLS-1$
} else {
String path = url.getPath();
matcher = PATH_PATTERN.matcher(path);
}
if (matcher.find()) {
return new Point(Integer.parseInt(matcher.group(1)), Integer.parseInt(matcher.group(1)));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldnt one of them be matcher.group(2) ?

@laeubi laeubi force-pushed the hint_size_for_dynamically_sized_images branch from fc3b77a to 123c69a Compare December 9, 2025 05:46
Currently Icons can be organized in folders like /icons/16x16,
/icons/32x32 and so on for high resolution support.
Currently if one want to replace such structure with SVG it is required
to scale down the SVG to 16x16 pixel document size as otherwise they get
rendered at there native size (what usually is much larger).

As it is not really desirable to restrict the size of the SVG design for
technical reasons, JFace now can detect two cases:

1) the SVG is places in a folder with "classic" folder layout the size
is extracted and passed down as a hint for dynamic sizable icons
2) one can additionally add a query parameter, e.g. if I have an icon
like /icons/obj16/search.svg and I have two places where I want to use
it one for a toolbar (16x16) and once for a Wizard Images (usually
128x128) I can use the url
bundle:/example.id/icons/obj16/search.svg?size=16x16 and
bundle:/example.id/icons/obj16/search.svg?size=128x128 to accomplish
this task without the need to even store two SVGs
@laeubi laeubi force-pushed the hint_size_for_dynamically_sized_images branch from 123c69a to 7a4c784 Compare December 9, 2025 06:56
@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:

tests/org.eclipse.jface.tests/META-INF/MANIFEST.MF

An additional commit containing all the necessary changes was pushed to the top of this PR's branch. To obtain these changes (for example if you want to push more changes) either fetch from your fork or apply the git patch.

Git patch
From 296a7771225554063f0aa4e723797297fda4a418 Mon Sep 17 00:00:00 2001
From: Eclipse Platform Bot <platform-bot@eclipse.org>
Date: Tue, 9 Dec 2025 07:01:35 +0000
Subject: [PATCH] Version bump(s) for 4.39 stream


diff --git a/tests/org.eclipse.jface.tests/META-INF/MANIFEST.MF b/tests/org.eclipse.jface.tests/META-INF/MANIFEST.MF
index b8dc172d3e..8cadbfa2c6 100644
--- a/tests/org.eclipse.jface.tests/META-INF/MANIFEST.MF
+++ b/tests/org.eclipse.jface.tests/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %Bundle-Name
 Bundle-SymbolicName: org.eclipse.jface.tests
-Bundle-Version: 1.4.1100.qualifier
+Bundle-Version: 1.4.1200.qualifier
 Automatic-Module-Name: org.eclipse.jface.tests
 Bundle-RequiredExecutionEnvironment: JavaSE-17
 Require-Bundle: org.junit;bundle-version="4.12.0",
-- 
2.52.0

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

@laeubi
Copy link
Contributor Author

laeubi commented Dec 9, 2025

Some test are failing here because Equinox is not parsing query strings correctly, I tried to address this 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.

4 participants