Community
Participate
Working Groups
I suggest to start using png in JDT ui which were contributed by Bug 385003.
Can't push to Gerrit in JDT UI, I use the following URL to push: ssh://lvogel@git.eclipse.org:29418/jdt/eclipse.jdt.ui.git Commit message is: ------------ Bug 426025 - Switch JDT UI to use png file instead of .gif Change-Id: I34840dd61ed609d95224b642b4316765564eb1fd Signed-off-by: Lars Vogel <Lars.Vogel@gmail.com> --------- Adding Thanh, maybe he can check.
What error are you getting?
(In reply to Thanh Ha from comment #3) > What error are you getting? That I did not sign the CLA.
Full message: --------------------- Repository ssh://lvogel@git.eclipse.org:29418/jdt/eclipse.jdt.ui.git The contributor must "sign-off" on the contribution. Processing changes: refs: 1 Processing changes: refs: 1, done ---------- Reviewing commit: commit 1e900a3f2bb70429f8b3514d454350202527b2dd 1389989640 ----sp Authored by: Lars Vogel <Lars.Vogel@gmail.com> The author is not a committer on the project. The author has a current Contributor License Agreement (CLA) on file. error: The author has not "signed-off" on the contribution. Please see http://wiki.eclipse.org/CLA -------------------- I signed of on the contribution, see Comment 2 for my commit message.
(In reply to Lars Vogel from comment #5) Lars, can you double check that you only have 1 commit you're trying to push? As far as I can tell from your 2 outputs everything looks ok. Unfortunately Gerrit typically only returns a error message for 1 of the commits so if there happens to be more than one in the chain and any of them fail the checks, you might be getting the log for a commit you were not suspecting.
(In reply to Thanh Ha from comment #6) > (In reply to Lars Vogel from comment #5) > > Lars, can you double check that you only have 1 commit you're trying to push? Thanks Thanh, silly me. I plan to ask our Gerrit friends to give an improved error message. Gerrit review (still depends on 426024): https://git.eclipse.org/r/20825
I would just live to add my vote for this pull-request to be merged. Several of the toolbar icons when using eclipse on OSX look very odd due the the background being darker than the anti-aliasing highlights in the icons.
s/live/like/
JDT team, any concerns with the icons contributed by Tony? He is very good in providing fixed icons but also very busy, so it would be great if you test or merge the icons and raise you concerns early so that we can fix any concern in time. Tony does this contribution in his free time, so early feedback would be great.
(In reply to Lars Vogel from comment #10) > JDT team, any concerns with the icons contributed by Tony? He is very good > in providing fixed icons but also very busy, so it would be great if you > test or merge the icons and raise you concerns early so that we can fix any > concern in time. > > Tony does this contribution in his free time, so early feedback would be > great. We'll try but we are still completely busy working on Java 8 features and fixes for Luna.
(In reply to Dani Megert from comment #11) > We'll try but we are still completely busy working on Java 8 features and > fixes for Luna. Maybe you can apply the patch and tell us during your work which icons are bad so that we can update them during your work?
(In reply to Lars Vogel from comment #12) > (In reply to Dani Megert from comment #11) > > We'll try but we are still completely busy working on Java 8 features and > > fixes for Luna. > > Maybe you can apply the patch and tell us during your work which icons are > bad so that we can update them during your work? I'd first like to settle down the other icons. I observed quite some fallout (recent one being bug 426110) and some waiting for response/fix (e.g. bug 430166).
Bug 432405 shows that the new PNGs aren't just the GIFs converted to PNG, but instead some new stuff seems to be tried out on some icons. Before we take the new PNGs, someone must compare each icon and confirm here, that they are identical.
(In reply to Dani Megert from comment #13) > (In reply to Lars Vogel from comment #12) > > (In reply to Dani Megert from comment #11) > > > We'll try but we are still completely busy working on Java 8 features and > > > fixes for Luna. > > > > Maybe you can apply the patch and tell us during your work which icons are > > bad so that we can update them during your work? > > I'd first like to settle down the other icons. I observed quite some fallout > (recent one being bug 426110) and some waiting for response/fix (e.g. bug > 430166). You cannot convert gif to png and improve the background. Tony has redrawn every single icon.
To add, they have to be redrawn if there are ever going be high-DPI versions of them.
(In reply to Timo Kinnunen from comment #16) > To add, they have to be redrawn if there are ever going be high-DPI versions > of them. No, Tony created svg versions and we generated the png files from them.
(In reply to Lars Vogel from comment #17) > No, Tony created svg versions and we generated the png files from them. Will the svg versions be contributed? (I don't see them on the Gerrit contribution)
(In reply to Wayne Beaton from comment #18) > Will the svg versions be contributed? Already are, see Bug 422139 for the current location and our plan to move that.
Dani / Markus, what is a good way to get these icons into JDT? Should I create one Gerrit review per icon / day so that you can maybe spend a small amount of time per day to review one icon?
(In reply to Lars Vogel from comment #20) > Dani / Markus, what is a good way to get these icons into JDT? Should I > create one Gerrit review per icon / day so that you can maybe spend a small > amount of time per day to review one icon? opentype.gif -> opentype.png https://git.eclipse.org/r/25704
Created attachment 242428 [details] Screenshot with comparison
class_obj.png https://git.eclipse.org/r/25705
(In reply to Lars Vogel from comment #20) > Dani / Markus, what is a good way to get these icons into JDT? Should I > create one Gerrit review per icon / day so that you can maybe spend a small > amount of time per day to review one icon? Best is to prepare one big Gerrit change which replaces the GIFs with the PNGs and where you already reviewed each PNG for being the same as the corresponding GIF and aren't washed out. We will look at such a Gerrit change in RC1 but not later.
(In reply to Dani Megert from comment #24) > (In reply to Lars Vogel from comment #20) > > Dani / Markus, what is a good way to get these icons into JDT? Should I > > create one Gerrit review per icon / day so that you can maybe spend a small > > amount of time per day to review one icon? > > Best is to prepare one big Gerrit change which replaces the GIFs with the > PNGs and where you already reviewed each PNG for being the same as the > corresponding GIF and aren't washed out. I have created separate Gerrit review for the "most important" JDT icons (the one visible in the toolbar). Can you review them? Should be fast as its only one icon per review. I update the rest within one go, I would like to get at least the toolbar right, and given your desire for perfect new icons, I think it is unlikely that we (Tony and I) can react fast enough if you review all at the same time. opentype.gif -> opentype.png https://git.eclipse.org/r/25704 class_obj.png https://git.eclipse.org/r/25705 For the other toolbar item I found it not good enough, see Bug 433717
(In reply to Lars Vogel from comment #25) This is the last time I even looked at a change set that is not based on master. Both icons are worse than master: - opentype.png has flatter balls and the folder is unsharp (but it at least looks the same as the new IFolder icon from the platform). I could accept this. - class_obj.png is definitely out. Just paste this into the Package Explorer and then check the Outline view and compare it to the "New Java Class" toolbar button: interface I {} class B {} public class C {} class D {} Furthermore, the Java type icons often have overlays, and we would have to verify that they look good in various situations.
(In reply to Markus Keller from comment #26) > (In reply to Lars Vogel from comment #25) > This is the last time I even looked at a change set that is not based on > master. Thanks for the review. I ask Tony to update.
Created attachment 242888 [details] newclass_wiz.png I'm not sure why you want to update class_obj.gif. Did you actually mean newclass_wiz.gif that is shown in the main toolbar? For that, we can just go with a png version like this that fixes the white corners but otherwise looks the same as the base icon. Note that all the Java type icons and new*_wiz would have to be updated together, and I don't think this will make it into 4.4.
Created attachment 249634 [details] icon compare gallery dec 27 I've attached a comparison gallery for JDT's new icons. There are two columns for light and dark background comparison. The gif is on the left, png on the right for each background color. Please review the icons for showstopper visual issues. I will regenerate the gallery as we fix issues. Once we're in a good place to do a real test, Lars will submit an updated patch for the ide. I have some initial code written to generate these galleries during rendering, which we'll use for initial QA on the remaining features.
(In reply to Tony McCrary from comment #29) Sorry for the late response Tony. If it's not too much work, could you generate that table as html, so that we can copy&paste the names of the icons that need polish? Most icons look good or even better than the corresponding GIF. One big remaining aspect is sharpness. I think for most of them you can see it yourself, e.g. toggle_breadcrumb, or most of the type icons. Also, the + adornment is less sharp than in the GIFs. In some of the icons the decorations are too strong, e.g. the @ in most icons attracts more attention than before. Some became smaller and unreadable, e.g. enum_tsk. The bended arrow in open_browser is not as good visible as in the GIF. Please review each icons for above mentioned problems. As Markus pointed out, once we're happy with the new icons, the next step is to try them out in the IDE, since many of those also get decorated dynamically.
Tony provided a reworked set of icons. @Tony can you attach a new side by side comparison for JDT to have a look? If JDT is ok with the new icons, I can create a Gerrit review for JDT.
Created attachment 252022 [details] Comparison of Gif and PNG Attached the rework of Tony, unfortunately creating a HTML page was not possible. There are two columns for light and dark background comparison. The gif is on the left, png on the right for each background color.
New Gerrit change created: https://git.eclipse.org/r/44943
Created attachment 252052 [details] Fixed JDT gallery Attached is an updated gallery that fixes the access_* icon in the beginning of the gallery. A new version of inkscape breaks compatibility with the batik svg renderer.
Gerrit change https://git.eclipse.org/r/44943 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=6ff7d5669cdf61acac539e3eda0fb8cc14553597
New Gerrit change created: https://git.eclipse.org/r/44959
Created attachment 252060 [details] Yet another update The attached update adds contrast lines for overlay icons (warning, static, etc).
Gerrit change https://git.eclipse.org/r/44959 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=c7489159f31f1be8c2dc1b3883d2e20cc38b14e9
(In reply to Tony McCrary from comment #37) > Created attachment 252060 [details] I feel Dani's comments from comment #30 are still valid for some icons. In general, arrows and letters need more work. Some specific improvements that can be made: - Arrows need to be sharper, e.g. in callee_co, caller_co etc. - annotation_tsk, class_tsk, enum_tsk, read icons are not as clear as before. - classf_generate, classfilegeneration_tab: "10" in 010 is not clearly visible in light background. In dark background, 010 is not visible at all compared to gif icons. - Not sure what is add_obj, but the plus looks big.
(In reply to Noopur Gupta from comment #39) > - Not sure what is add_obj, but the plus looks big. I think this was added by accident, removed with https://git.eclipse.org/r/#/c/44973/
New Gerrit change created: https://git.eclipse.org/r/45012
The above commit integrates the current JDT png icons into the jdt.ui bundle. Please try them out in the IDE and then list which ones need further work. In the spirit of improving the visual appearance of Eclipse, we can replace the icons that pass for Mars and then get the failed icons into a future update (or possibly Mars if there is time). This doesn't include debug.ui, I need to add icons for that bundle.
New Gerrit change created: https://git.eclipse.org/r/45015
I noticed an issue with the previous submission, please use this one: https://git.eclipse.org/r/45015
Also please test on both Class and Dark themes.
(In reply to Tony McCrary from comment #42) > The above commit integrates the current JDT png icons into the jdt.ui bundle. > > Please try them out in the IDE and then list which ones need further work. Did you address Noopurs feedback with this update?
No, it's only updates to the jdt.ui bundle itself for testing inside the IDE.
(In reply to Tony McCrary from comment #47) > No, it's only updates to the jdt.ui bundle itself for testing inside the IDE. I assume the JDT UI hopes that the known issues are resolved before integrating the new icons into the code base.
Does this really need to be a monolithic "all or nothing" update? The icons use the same style, so if some are gif and some are png it should look okay. In any event, we need to test the dynamic overlay image functionality in the IDE as mentioned earlier. The gerrit change makes it easy to test out.
It would be difficult and tedious to have a mix with partial commits as it is also reflected in the code, hence a complete set is expected.
New Gerrit change created: https://git.eclipse.org/r/45903
Created attachment 252439 [details] JDT Gallery April 15 2015
Created attachment 252440 [details] Gallery April 15 2015 - 2
(In reply to Eclipse Genie from comment #51) > New Gerrit change created: https://git.eclipse.org/r/45903 This doesn't look like the complete patch. Also, noticed that if you have a 'static final' field, then icon for "S" (static_co) looks too bold compared to "F". And, the "C" in class_obj looks like "O" when seen in the IDE.
(In reply to Noopur Gupta from comment #54) > (In reply to Eclipse Genie from comment #51) > > New Gerrit change created: https://git.eclipse.org/r/45903 > > This doesn't look like the complete patch. This was a Gerrit review for the icon repo, merged via Bug 464877. I assume Tony waits with the JDT review until he gets the OK for the new icons.
Oddities found when I imported /org.eclipse.ui.images/eclipse-png/org.eclipse.jdt.ui/icons/full into org.eclipse.jdt.ui/icons/full: - dview16 is unnecessary (view icons are never rendered disabled) - the arrow in open_browser.png and the pin in pin_view.png are hard to recognize - the arrows in cp_order_obj.png should not touch each other (the right one can be moved 1 px to the right) - newjprj_wiz.png looks quite different from the "New General Project" icon from the platform. The base icon should be the same. Missing icons in master of /org.eclipse.ui.images/eclipse-png/org.eclipse.jdt.ui: - obj16/external_annotation_location_attrib.gif - ovr16/default_co.gif Added to org.eclipse.ui.images with: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=7e03a61697ac3652c8897cbd9b5b0ebba467afc4 Removed wrongly added files: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=0ee3cec7017426188771ac186c95ce4085e14762
Every icon gets a disabled variant when rendered (the renderer has no idea how the icons are actually used). To clarify about newjprj_wiz.png, you are saying you'd prefer the icon to look different from the existing JDT gif and more like the new general project icon used in platform?
(In reply to Tony McCrary from comment #57) > Every icon gets a disabled variant when rendered (the renderer has no idea > how the icons are actually used). Yes, that's OK. But we still won't commit the dview16 folder, since we know that all icons in that folder are unused. > To clarify about newjprj_wiz.png, you are saying you'd prefer the icon to > look different from the existing JDT gif and more like the new general > project icon used in platform? The current newprj_wiz.png is actually quite far off the original newprj_wiz.gif and should never have been released in this form: - the star is blurry and at a wrong position - the folder is at a wrong position - the folder has lost the "hangers" (horizontal bar on top and on the left of the sheet that points towards the screen). The original metaphor was that a project is a specially huge hanging folder that can contain sub-folders. The cprj_obj.png still has the hangers, but prj_obj.png also misses them. prj_obj.png and newprj_wiz.png should be fixed first, and then JDT's newjprj_wiz.png should be based on the fixed newprj_wiz.png.
Created attachment 252611 [details] April 21 2015 Gallery I've updated the icons you've mentioned. I'm still trying to get the browser arrow to work well when rasterized at low resolutions, but it's better than before. The Class C has been changed so that it doesn't look closed in certain areas (the "mouth" is a few pixels wider now). The platform new project icon has also been updated to match JDT.
(In reply to Tony McCrary from comment #59) Looks good, thank you! (In reply to Markus Keller from comment #56) > Missing icons in master of > /org.eclipse.ui.images/eclipse-png/org.eclipse.jdt.ui: > - obj16/external_annotation_location_attrib.gif > - ovr16/default_co.gif -- caveat: this is the letter "D", not to be confused with the blue triangle elcl16/default_co.gif These are the only remaining blockers for JDT UI. Once we have a Gerrit review or commit that includes these icons, we can have a final look at the overlays and then commit for M7.
Created attachment 252660 [details] April 22 2015 Gallery Here's the latest gallery with the missing icons and other fixes. I finally found a way to create the ovr art that looks decent when scaled up (i.e. not svg pixel art).
New Gerrit change created: https://git.eclipse.org/r/46289 WARNING: this patchset contains 1852 new lines of code and may require a Contribution Questionnaire (CQ) if the author is not a committer on the project. Please see:https://wiki.eclipse.org/Project_Management_Infrastructure/Creating_A_Contribution_Questionnaire
I consistently get errors when trying to render the updated /org.eclipse.ui.images/eclipse-svg/org.eclipse.jdt.ui/icons/full/elcl16/external_browser.svg and open_browser.svg with the Maven build. You seem to get the same error, since the latest Gerrit change misses updates for these icons as well. Apart from the rendering bug, the arrow's tail should still be a bit thicker. The head looks good now, but the tail is still hard to see in 16x16. [INFO] pool-1-thread-5 Rasterizing: open_browser.png at 16x16 org.apache.batik.bridge.BridgeException: file://open_browser.svg:-1 The attribute "r" of the element <circle> is required at org.apache.batik.bridge.SVGCircleElementBridge.buildShape(SVGCircleElementBridge.java:93) at org.apache.batik.bridge.SVGShapeElementBridge.createGraphicsNode(SVGShapeElementBridge.java:60) at org.apache.batik.bridge.GVTBuilder.buildGraphicsNode(GVTBuilder.java:213) at org.apache.batik.bridge.GVTBuilder.buildComposite(GVTBuilder.java:171) at org.apache.batik.bridge.GVTBuilder.buildGraphicsNode(GVTBuilder.java:219) at org.apache.batik.bridge.GVTBuilder.buildComposite(GVTBuilder.java:171) at org.apache.batik.bridge.GVTBuilder.buildGraphicsNode(GVTBuilder.java:219) at org.apache.batik.bridge.GVTBuilder.buildComposite(GVTBuilder.java:171) at org.apache.batik.bridge.GVTBuilder.build(GVTBuilder.java:82) at org.apache.batik.transcoder.SVGAbstractTranscoder.transcode(SVGAbstractTranscoder.java:208) at org.apache.batik.transcoder.image.ImageTranscoder.transcode(ImageTranscoder.java:92) at org.apache.batik.transcoder.XMLAbstractTranscoder.transcode(XMLAbstractTranscoder.java:142) at org.apache.batik.transcoder.SVGAbstractTranscoder.transcode(SVGAbstractTranscoder.java:156) at org.eclipse.ui.images.renderer.RenderMojo.renderIcon(RenderMojo.java:440) at org.eclipse.ui.images.renderer.RenderMojo.rasterize(RenderMojo.java:184) at org.eclipse.ui.images.renderer.RenderMojo$1.call(RenderMojo.java:335) at java.util.concurrent.FutureTask.run(FutureTask.java:266) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617) at java.lang.Thread.run(Thread.java:745)
Created attachment 252697 [details] April 23 2015 Gallery Updated gallery, commit will be next.
New Gerrit change created: https://git.eclipse.org/r/46386
Created attachment 252698 [details] April 23 2015 Gallery - CORRECTED This is the correct latest gallery (I forgot to rerun the gallery mojo on the last one)
(In reply to Tony McCrary from comment #66) Thanks, that looks great. I'll integrate it tomorrow. I wonder why the PNG generation is not deterministic. When I run this multiple times locally, then I always get the same PNG file contents (but different from your last Gerrit change): $ $ mvn -Declipse.svg.scale=1 -Declipse.svg.renderthreads=1 org.eclipse.ui:org.eclipse.ui.images.renderer:render-icons When I run with the default thread count (8), then the build is a few seconds quicker, but on the other hand, a few of the generated images look slightly different. Looks like it mostly affects disabled icons. I'm afraid at least the ContrastFilter is just not multi-threading capable, and the initialization problem I fixed recently was just the tip of the iceberg. But that still doesn't explain why most images are rendered differently over here. Could it be a platform-dependency? I'm on Mac OS X 10.10.
I'm not sure, we use the svg images directly in our product. Apache Batik's transcoder uses Java2D to render the svg images and if I recall Apple implemented AWT, Java2D, Swing on their own, so perhaps that's where the differences come from. I only use Linux when working on this artwork. It's funny that the contrast filter isn't thread safe, really the operation itself shouldn't require any state to be shared between invocations.
Out of curiosity, could you post some of the more noticeable OSX differences?
Gerrit change https://git.eclipse.org/r/46386 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=ada0546ae4aa696282dab6c75190df83e5823269
(In reply to Eclipse Genie from comment #65) > New Gerrit change created: https://git.eclipse.org/r/46386 Images merged via Bug 465352.
Created attachment 252730 [details] dlcl16/debug_view_tree.png (artifacts from multi-thread rendering) Here's an example with 3 rendering artifacts (darker pixels) from multi-thread rendering. As for the different output files when I render them locally: I can't see any differences in any of the rendered files. The bits are not identical, but the locally rendered images look equivalent to yours.
Yeah I've noticed this before. The easiest thing to do would be to move filter creation into each worker execution itself. That way they each have their own state and the filtering operations should behave like single threaded mode. Concurrency is easy if there's no shared state, it's a shame the jhlabs filters weren't designed with this in mind as they don't really *need* shared state. Performance may suffer a bit but it's not that important (more time is probably spent starting up java, maven and waiting on IO). I suppose we could patch the filters so they are synchronized when initializing some of their LUT data but is that really worth the time spent? WRT to the OSX files, I think there are many variables in play: * Floating point math and different OSes and hardware * Various stochastic functions in the SVG renderer for different phenomenon * Possibly JVM/JIT differences (I primarily use the latest debian stable version of OpenJDK 7 ATM) * Different PNG metadata being encoded (system name, username, timestamp, etc) * Bugs in stdlib/java/java2d/batik/RenderMojo, etc.
I've kept these 4 .gif files: /org.eclipse.jdt.ui/icons/full/obj16/classf_obj.gif | jcu_obj.gif | file_obj.gif | jar_desc_obj Apparently, E4 stores a reference to the icons of "org.eclipse.ui.editors" contributions and uses them to render the icons in tabs for restored editors. Filed bug 465456. Switched all icons used by JDT UI to .png and removed the other .gif files. Kudos to Tony for drawing all the icons in SVG! Fixed with http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=464f603228cbed4a27826b49f03739be08c691bf
I think this is noteworthy. JDT team please remove the tag if you disagree.
(In reply to Lars Vogel from comment #75) > I think this is noteworthy. JDT team please remove the tag if you disagree. You can add one entry for the new PNGs that mention JDT, PDE and other places where PNGs are present now.
Verified in 4.5.0.N20150425-1500, JDT icons look way better in the dark theme.
After updating an existing eclipse install to I20150428-0100 all icons of minimized JDT views are broken (red square). Seen with - Javadoc - Declarations - Call Hierarchy The first time such a view is restored, the icon comes back.
Sounds like E4 caches even more. I'm investigating and I'll probably restore the view icons as well and update bug 465456.
(In reply to Stephan Herrmann from comment #78) > After updating an existing eclipse install to I20150428-0100 all icons of > minimized JDT views are broken (red square). Seen with Thanks, fixed by restoring /org.eclipse.jdt.ui/icons/full/eview16/*.gif for the broken E4 model persistence implementation (bug 465456): http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=f73a82be267041be731ae9771c8c920a83a6c06b
