Community
Participate
Working Groups
I open this bug for the work to use the new images provided by org.eclipse.ui.images.
To support alpha transparency we should switch to the donated png files with real alpha transparency. I start with org.eclipse.ui, seems to be the most visible.
We have 165 GIFs in org.eclipse.ui based on: find icons -type f | wc -l
Sorry, also some bmp files, currently we have 135 GIF files find icons -name "*.gif" -type f | wc -l org.eclipse.ui.images/eclipse-png/org.eclipse.ui only has 100 png files. Looks like some are missing.
Tony, is this difference expected?
Looks to me that the "d" icons are missing. Maybe an error in the PNG generator? dlcl16 dtool16
(In reply to Lars Vogel from comment #5) > Looks to me that the "d" icons are missing. Maybe an error in the PNG > generator? > > dlcl16 > dtool16 Could be an error. The way it's currently handled is pretty brittle. I will get that fixed.
(In reply to Tony McCrary from comment #6) > (In reply to Lars Vogel from comment #5) > > Looks to me that the "d" icons are missing. Maybe an error in the PNG > > generator? > > > > dlcl16 > > dtool16 > > Could be an error. The way it's currently handled is pretty brittle. > > I will get that fixed. The issue is that it only generates disabled icons if there's already a disabled folder in the parent directory. So if etool16 exists, it checks if dtool16 exists. If dtool16 exists, it creates the disabled icons. I believe this was originally done because not all e-prefixed folders had disabled variants. I can change it so if there's an e-prefixed folder, it creates the disabled variants regardless of a preexisting disabled folder. This may generate some disabled PNG icons that aren't currently used, but it may make sense to have a disabled version of every "enabled" icon anyway just in case.
+1 for always creating the d version if we have an enabled one.
I've submitted a change that always creates disabled variants for e-prefixed icon directories: https://git.eclipse.org/r/#/c/18624/
(In reply to Tony McCrary from comment #9) > I've submitted a change that always creates disabled variants for e-prefixed > icon directories: > > https://git.eclipse.org/r/#/c/18624/ Sorry about the tabs, my "Insert spaces for tabs" is apparently not working through checked.
Thanks. After the change I end up with 161 generated images. Looks good, I check tomorrow more in detail.
Created attachment 237611 [details] Save is really small I tested the usage of the generated png files but they get displayed really small. See attached screenshot. The GIF and PNG file is 16x16 therefore I'm not sure what is causing the difference.
I noticed that org.eclipse.ui/obj16/blank.gif has no replacement. Tony, could you add an empty svg for that?
https://git.eclipse.org/r/#/c/18624/ for the missing disabled icons.
(In reply to Lars Vogel from comment #12) > Created attachment 237611 [details] > Save is really small > > I tested the usage of the generated png files but they get displayed really > small. See attached screenshot. The GIF and PNG file is 16x16 therefore I'm > not sure what is causing the difference. Interesting. So far I can't duplicate this in other software, pngcheck outputs the following: $ pngcheck -vt save_edit.png File: save_edit.png (933 bytes) chunk IHDR at offset 0x0000c, length 13 16 x 16 image, 32-bit RGB+alpha, non-interlaced chunk IDAT at offset 0x00025, length 876 zlib: deflated, 32K window, maximum compression chunk IEND at offset 0x0039d, length 0 No errors detected in save_edit.png (3 chunks, 8.9% compression). So I'm leaning toward a possible SWT/gtk+ bug, but I will try to rule out ImageIO being the culprit.
(In reply to Tony McCrary from comment #15) > So I'm leaning toward a possible SWT/gtk+ bug, but I will try to rule out > ImageIO being the culprit. I think this was my fault, I had a missing icon for the new project wizard and this seems to cause the toolbar rendering to fail. I also realized that blank.gif is... blank. While my graphical skills are very limited, I can create an empty svg.
Added the blank.png with https://git.eclipse.org/r/#/c/18680/ Committed with http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=a7b4b6f72e36778f0a58a4727a541c82222a5f0c
Created attachment 237626 [details] Screenshot with new icons
Here is the change for org.eclipse.ui and org.eclipse.ui.workbench https://git.eclipse.org/r/18685 The screenshot I attached shows that the new icons looks also good on a black background.
(In reply to Lars Vogel from comment #19) > Here is the change for org.eclipse.ui and org.eclipse.ui.workbench > > https://git.eclipse.org/r/18685 > > The screenshot I attached shows that the new icons looks also good on a > black background. Awesome, thanks. Some of the new icons seem slightly translucent, is that the icon or something with the render?
> Awesome, thanks. Some of the new icons seem slightly translucent, is that > the icon or something with the render? As far as I can tell, it just looks like this on the black background.
Lars, https://git.eclipse.org/r/#/c/18685 causes a failing test in UiTestSuite. junit.framework.AssertionFailedError at junit.framework.Assert.fail(Assert.java:55) at junit.framework.Assert.assertTrue(Assert.java:22) at junit.framework.Assert.assertTrue(Assert.java:31) at org.eclipse.ui.tests.harness.util.ImageTests.assertEquals(ImageTests.java:35) at org.eclipse.ui.tests.api.EditorIconTest.testBadIcon(EditorIconTest.java:86) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:606) at junit.framework.TestCase.runTest(TestCase.java:176) at junit.framework.TestCase.runBare(TestCase.java:141) at junit.framework.TestResult$1.protect(TestResult.java:122) at junit.framework.TestResult.runProtected(TestResult.java:142) at junit.framework.TestResult.run(TestResult.java:125) at junit.framework.TestCase.run(TestCase.java:129) at junit.framework.TestSuite.runTest(TestSuite.java:255) at junit.framework.TestSuite.run(TestSuite.java:250) at junit.framework.TestSuite.runTest(TestSuite.java:255) at junit.framework.TestSuite.run(TestSuite.java:250) at junit.framework.TestSuite.runTest(TestSuite.java:255) at junit.framework.TestSuite.run(TestSuite.java:250) at org.eclipse.jdt.internal.junit.runner.junit3.JUnit3TestReference.run(JUnit3TestReference.java:131) at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38) at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:459) at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:675) at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:382) at org.eclipse.pde.internal.junit.runtime.RemotePluginTestRunner.main(RemotePluginTestRunner.java:62) at org.eclipse.pde.internal.junit.runtime.PlatformUITestHarness$1.run(PlatformUITestHarness.java:47) at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:35) at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:136) at org.eclipse.swt.widgets.Display.runAsyncMessages(Display.java:3735) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3384) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$9.run(PartRenderingEngine.java:1113) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:997) at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:146) at org.eclipse.ui.internal.Workbench$5.run(Workbench.java:613) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332) at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:567) at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:150) at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:124) at org.eclipse.pde.internal.junit.runtime.NonUIThreadTestApplication.runApp(NonUIThreadTestApplication.java:54) at org.eclipse.pde.internal.junit.runtime.UITestApplication.runApp(UITestApplication.java:47) at org.eclipse.pde.internal.junit.runtime.NonUIThreadTestApplication.start(NonUIThreadTestApplication.java:48) at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:196) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:109) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:80) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:372) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:226) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:606) at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:636) at org.eclipse.equinox.launcher.Main.basicRun(Main.java:591) at org.eclipse.equinox.launcher.Main.run(Main.java:1450) at org.eclipse.equinox.launcher.Main.main(Main.java:1426)
(In reply to Paul Webster from comment #22) > Lars, https://git.eclipse.org/r/#/c/18685 causes a failing test in > UiTestSuite. I fixed the failing tests, change was required in EditorIconTest and in tests/org.eclipse.ui.tests/plugin.xml https://git.eclipse.org/r/#/c/18685 is updated
(In reply to Lars Vogel from comment #23) > > https://git.eclipse.org/r/#/c/18685 is updated Released as http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=409f9bf91a1ce1a61b1430a0f60ed187ee624604 PW
Created attachment 238215 [details] Screenshot 2 The new icons look quite blurry, e.g. the + on wizards is hardly recognizable as a + any more.
Lars, can we get sharper images generated by the tycho plugin? PW
Created attachment 238216 [details] wizard icon 64x64
(In reply to Markus Keller from comment #25) > Created attachment 238215 [details] > Screenshot 2 > > The new icons look quite blurry, e.g. the + on wizards is hardly > recognizable as a + any more. The + isn't a plus, it's a star (i.e. new wizard, magic, etc). Perhaps we could add some darker elements to make it look more like the original. The original icons were pixel art. The arrow isn't blurry, it has antialiasing so it isn't jagged (look at it in an image editor). We could render it without AA, which would probably look terrible. It may also be possible to tweak the resampling process a bit.
I agree looks a bit blurry, Tony maybe we can use the same decoration as the one used for the other elements, e.g. the new Folder?
(In reply to Lars Vogel from comment #29) > I agree looks a bit blurry, Tony maybe we can use the same decoration as the > one used for the other elements, e.g. the new Folder? Sure, shouldn't be a problem. One of the tricks with "crispness" and SVG is being aware of pixel alignment, even though SVG is resolution independent. You have to make sure lines and other elements end on pixel boundaries or they get averaged between two pixels and you get blur. There are still some icons in the new set that need some tweaking in this regard. However, this requirement will go away with the new generation of high resolution monitors.
(In reply to Tony McCrary from comment #30) > One of the tricks with "crispness" and SVG is being aware of pixel > alignment, even though SVG is resolution independent. You have to make sure > lines and other elements end on pixel boundaries or they get averaged > between two pixels and you get blur. There are still some icons in the new > set that need some tweaking in this regard. However, this requirement will > go away with the new generation of high resolution monitors. I think the usage of the gradients in the SVG originals might also result in that blur effect. Markus, Paul: shall we revert the commit until we can created "crisp" icons or shall we leave it in and incrementally improve the icons?
(In reply to Lars Vogel from comment #31) > (In reply to Tony McCrary from comment #30) > > One of the tricks with "crispness" and SVG is being aware of pixel > > alignment, even though SVG is resolution independent. You have to make sure > > lines and other elements end on pixel boundaries or they get averaged > > between two pixels and you get blur. There are still some icons in the new > > set that need some tweaking in this regard. However, this requirement will > > go away with the new generation of high resolution monitors. > > I think the usage of the gradients in the SVG originals might also result in > that blur effect. > > Markus, Paul: shall we revert the commit until we can created "crisp" icons > or shall we leave it in and incrementally improve the icons? I'm thinking about rewriting the renderer to use Inkscape's batch interface. I've done a comparison and the quality is much better than batik. This would require installing inkscape before rendering the icons but it's open source and works on Linux, Mac and Windows. What do you think? Inkscape is an open source SVG editor (which was used to create the icons). http://inkscape.org/en/
Created attachment 238230 [details] Inkscape rendered icon
After digging into Batik a bit, I think I may have found why Batik's output has been so strange at low resolutions (or at least how to prevent that).
(In reply to Tony McCrary from comment #34) > After digging into Batik a bit, I think I may have found why Batik's output > has been so strange at low resolutions (or at least how to prevent that). Thanks Tony for looking into it. I think both options (a better Batik or the usage of Inkscape) would be ok, as the generation of the png files is anyway a step which we would trigger manually.
(In reply to Tony McCrary from comment #33) > Created attachment 238230 [details] > Inkscape rendered icon This looks much better. (In reply to Lars Vogel from comment #31) > Markus, Paul: shall we revert the commit until we can created "crisp" icons > or shall we leave it in and incrementally improve the icons? I would ship M4 with the old gifs and then use M5 to improve the icons all over the SDK. We already know we will get better icons, so shipping M4 with blurry icons could just lead to unnecessary swirl.
(In reply to Markus Keller from comment #36) > (In reply to Tony McCrary from comment #33) > > Created attachment 238230 [details] > > Inkscape rendered icon > > This looks much better. > > (In reply to Lars Vogel from comment #31) > > Markus, Paul: shall we revert the commit until we can created "crisp" icons > > or shall we leave it in and incrementally improve the icons? > > I would ship M4 with the old gifs and then use M5 to improve the icons all > over the SDK. We already know we will get better icons, so shipping M4 with > blurry icons could just lead to unnecessary swirl. +1. M5 gives us enough time to implement this in more components than just the platform.
(In reply to Dani Megert from comment #37) > +1. M5 gives us enough time to implement this in more components than just > the platform. +1, can someone revert the commit, I'm currently in a customer workshop and don't think a late night commit for this is a good thing.
(In reply to Lars Vogel from comment #38) > (In reply to Dani Megert from comment #37) > > +1. M5 gives us enough time to implement this in more components than just > > the platform. > > +1, can someone revert the commit, I'm currently in a customer workshop and > don't think a late night commit for this is a good thing. Done with http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=8668d206f87bf9d1b696d7f14072cdb510aebdf5
(In reply to Dani Megert from comment #39) > Done with > http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/ > ?id=8668d206f87bf9d1b696d7f14072cdb510aebdf5 Thanks a lot.
Created attachment 238527 [details] Updated icon for comparison
I've submitted a contribution that improves batik rasterization output. The above attachment has the blurry icon on the left, the middle is the icon rendered with new code and the left icon is the original gif image. Let me know what you think.
That should have been "and the *right* icon is the original gif image."
(In reply to Tony McCrary from comment #43) > That should have been "and the *right* icon is the original gif image." New icon looks way better IMHO. Can you post the link to your new generator? I assume you uploaded it to Gerrit and it is a enhancement of your Maven mojo?
(In reply to Lars Vogel from comment #44) > (In reply to Tony McCrary from comment #43) > > That should have been "and the *right* icon is the original gif image." > > New icon looks way better IMHO. Can you post the link to your new generator? > I assume you uploaded it to Gerrit and it is a enhancement of your Maven > mojo? That includes the revised rasterizer mojo, cleaned up SVG sources, EMF star icon set and the new save icon: https://git.eclipse.org/r/#/c/20145/ This commit is the re-rendered set of icons: https://git.eclipse.org/r/#/c/20146/
(In reply to Tony McCrary from comment #45) > (In reply to Lars Vogel from comment #44) > > (In reply to Tony McCrary from comment #43) > > > That should have been "and the *right* icon is the original gif image." > > > > New icon looks way better IMHO. Can you post the link to your new generator? > > I assume you uploaded it to Gerrit and it is a enhancement of your Maven > > mojo? > > That includes the revised rasterizer mojo, cleaned up SVG sources, EMF star > icon set and the new save icon: > https://git.eclipse.org/r/#/c/20145/ If I run mvn clean install I get a syntax error, see the Gerrit review for the details. Anything which I need to install before running it?
(In reply to Lars Vogel from comment #46) > (In reply to Tony McCrary from comment #45) > > (In reply to Lars Vogel from comment #44) > > > (In reply to Tony McCrary from comment #43) > > > > That should have been "and the *right* icon is the original gif image." > > > > > > New icon looks way better IMHO. Can you post the link to your new generator? > > > I assume you uploaded it to Gerrit and it is a enhancement of your Maven > > > mojo? > > > > That includes the revised rasterizer mojo, cleaned up SVG sources, EMF star > > icon set and the new save icon: > > https://git.eclipse.org/r/#/c/20145/ > > If I run mvn clean install I get a syntax error, see the Gerrit review for > the details. Anything which I need to install before running it? Looks like I didn't include the updated pom, it needs: <dependency> <groupId>org.apache.xmlgraphics</groupId> <artifactId>batik-rasterizer</artifactId> <version>1.7</version> </dependency> instead of the current rasterizer. The newer version includes the ability to set rendering hints, which is required for generating low res targets. I didn't realize there was an issue because we don't need the renderer for regular builds, so Hudson completed successfully. I will create a new commit.
(In reply to Tony McCrary from comment #47) > (In reply to Lars Vogel from comment #46) > > (In reply to Tony McCrary from comment #45) > > > (In reply to Lars Vogel from comment #44) > > > > (In reply to Tony McCrary from comment #43) > > > > > That should have been "and the *right* icon is the original gif image." > > > > > > > > New icon looks way better IMHO. Can you post the link to your new generator? > > > > I assume you uploaded it to Gerrit and it is a enhancement of your Maven > > > > mojo? > > > > > > That includes the revised rasterizer mojo, cleaned up SVG sources, EMF star > > > icon set and the new save icon: > > > https://git.eclipse.org/r/#/c/20145/ > > > > If I run mvn clean install I get a syntax error, see the Gerrit review for > > the details. Anything which I need to install before running it? > > Looks like I didn't include the updated pom, it needs: > > <dependency> > <groupId>org.apache.xmlgraphics</groupId> > <artifactId>batik-rasterizer</artifactId> > <version>1.7</version> > </dependency> > > instead of the current rasterizer. The newer version includes the ability to > set rendering hints, which is required for generating low res targets. I > didn't realize there was an issue because we don't need the renderer for > regular builds, so Hudson completed successfully. > > I will create a new commit. Here are the new commits: New renderer code: https://git.eclipse.org/r/#/c/20198/ SVG updates + PNG renders https://git.eclipse.org/r/#/c/20199/ I've split them differently, the first commit has only the java+maven changes. The second commit has updated SVG icons and associated PNG renders for them. Let me know if you have any issues building this, it should work now.
(In reply to Tony McCrary from comment #48) > New renderer code: > https://git.eclipse.org/r/#/c/20198/ > > SVG updates + PNG renders > https://git.eclipse.org/r/#/c/20199/ Applied with https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=a37c3b7a9013b6cf8d9e3a2664a289384bc884c2 https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=fc23025fa1fc6875eb7bee2f9fbb0e32fe98c932 https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=41a08327f5e29418496b7bf1e519877f7e68df47
Latest generated png files uploaded to org.eclipse.ui.images with https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=95c848a94a60fe876e59a450f032bcb8924fe8fe
*** Bug 421292 has been marked as a duplicate of this bug. ***
Started to apply the changes again with the newly rendered icons: https://git.eclipse.org/r/20741
Fixed with https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=9d9b497fb4739bdf9e35f7759707f6f623c084d9 Thanks Tony for all the help!
Verified in N20140118-1500
