Donate more SVG icons, and install more duplicates #7463
Conversation
This commit adds or touches SVG files only, and includes no code changes. The Adobe Illustrator file that was used to generate the SVG files will be committed separately, along with the script that was used to find duplicates and copy the files to the right locations. Details: * Add new SVG icons drawn by myself: * Action icons for Cut, Copy, Paste, Notifications, Output, and Properties. * Speech bubble variations used to display notifications in the toolbar. * error/aborted/info/ok/question icons that are used in various places. * Additional icons included: lightbulb, globe, newFolder, configBadge8x8, print, hierarchy, restart, runProject * Regenerate and refresh all SVG icons that came from the same Illustrator file; some of these received minor tweaks since they were last committed. * For each of the currently drawn SVG icons, do a thorough search among existing GIF and PNG icons to find either identical or visual duplicates, and copy in the corresponding SVG icon in each place. (In a separate commit I will include a generated HTML file that shows an overview of these files.)
eirikbakke
added
Platform
neilcsmith-net
added
the
ci:dev-build
|
lets move this to NB24 |
|
Sure, no hurry! |
would have been cool if this could have made it in, but nobody looked at it so far and it is a large PR. I have also only 2x 1080p screens here so I can't simulate scaling very well. Maybe split this into two commits, one for modified icons one for added? The added icons are not used at the moment, right? |
|
The simplest way to test would most probably to just create a dev build and use it for a few days. And yes, I should have done it sooner. I don't think it is necessary to check each icon. We have much more invasive changes with less review. |
sure but those are also the PRs which often get reverted (and I hate doing that!). I think this here is actually reviewable, this looks like half of the changes are new icons. So looking through the modified icons quickly should be possible with the right tool or process. Humans can interpret pictures quite fast. (gh is not the right tool as you can see)
agreed. We could merge this shortly NB 23 is done, this would give enough time to find issues if there are any before NB 24. |
This is off-topic, but our rate of reverted PRs is low. The core problem from my POV is, that people don't use their own changes, else they would see the problems they introduce. In this case these icons were used in a real product and sure, shit happens, but lets assume good faith. |
They are actually used: ImageUtilities.loadImage automatically loads any SVG file that exists with the same name (but different extension) as the requested GIF or PNG icon. (So I wouldn't split up into two commits if this was the reason for doing so.)
This is what the HTML page generated by this script is for:
I've been using a NetBeans IDE build with these changes applied for a month now. Others are welcome to do the same, of course.
For testing purposes, just seeing that icons are not "out of whack" (obviously missing, errors loading etc.) is probably good enough. The new icons are used at 100% scaling, too, just at a lower resolution. |
|
@eirikbakke thanks for the info. The generated page is super useful. How does NB render the vector graphics atm, with batik? I think they look good, but you can definitively see the slight "style" difference if there is a svg/png mix in the same toolbar. But I saw that you already prioritized neighboring icons which is great. |
Yes, with Batik, originally introduced in this PR. Alternative approaches, such as storing each icon as a bitmap at 2x size, could be considered in the future, if we want to get rid of this rather heavyweight library.
Yeah, I tried to establish a style that made the mix between old and new icons reasonably tolerable. The main difference is the higher resolution in the newer icons, as well as the removal of bevels and shadows (which are hard to draw in vector graphics and are out of fashion for the same reason). |
Or we consider JSVG like FlatLaf and IntelliJ. We need to consider whether Batik is now a platform API, or whether just the ability to load from an SVG is (with potential for minor incompatibilities in changing implementation). |
|
I was in favour of merging this for NB23, which was why I added the milestone and dev build. I also didn't have time to review it properly before freeze either! So NB24 is fine IMO. @eirikbakke please put milestones on PRs and push people for review if necessary - I noticed a bunch of useful platform updates from you, like this, that were getting stale. |
I think we could merge this pretty soon (e.g after NB23rc1 and the version bump), since it is not very likely that icons will cause conflicts between delivery/master. The benefit of pushing this to NB24 is also that maybe we get a few more icons in to reduce the problem of #7463 (comment) (followup PRs). |
|
so, should we merge this? Maybe rebase first to latest master to make the merge commit smaller? cc @ebarboni |
Yes, that should be possible. Some visual comparisons would have to be made to ensure that the rendered SVGs look the same when rendered with a different library. (I assume the lighter libraries implement a smaller subset of the SVG standard than Batik.)
The SVG loader was intentionally made into an optional and pluggable ServiceProvider module, implementing org.openide.util.spi.SVGLoader in the openide.util.ui module. The current implementation, based on Batik, is in the optional openide.util.ui.svg module.
OK, will do this more in the future.
I think there will always end up being a mix of old and new icons, due to the sheer number of them (it's not just the Test Results tab, but also Search Results, Notifications, Output + all the debugging ones etc.). But getting work gradually merged in makes it easier to do incremental work later. |
matthiasblaesing
left a comment
matthiasblaesing
left a comment
There was a problem hiding this comment.
So I ran the last two weeks with this applied and looked at a random selection of icons. The changes look sane and new icons also seem to match there meaning.
I noticed in at least one occasion that an icon got a different visual (the "earth" icon for web) and at first I had a double take on it, but that smoothed and I'm currently happy with this.
Thank you.
|
Thanks for testing!
In this example it was still an "earth" icon before, right, not a different logo of some kind? (My Bezier curve drawing skills are somewhat limited... so the new earth may have had a somewhat improvised landmass.) -- Eirik |
Actually it was the color that triggered my reaction. Before it was blue-white (left), now it is blue-green (right):
Both tell the right story and with blue selection highlight, the earth with green continent has a nicer visual than the white. TL;DR: This was just a random observation and not intended to spark a discussion 😄 |
|
Great, yes, I just wanted to make sure there were no errors in the filename-to-artboard mapping logic. Old and new icons may look different in cases where there are many previous variations of the same motif. E.g. there were many different "globe" variations ( |
Creator of JSVG here. If you aren’t using any extremely complex filters or CSS declarations, JSVG should support all features you need. Though if you experience there is something missing feel free to open an issue. If I know what needs to be done I can certainly prioritise things. |
|
@weisJ Oh, thanks for dropping by! These SVGs are generated by Adobe Illustrator, I can test them out with JSVG at some point. There are some SVG export settings that can be adjusted in Illustrator if necessary, too. |
|
@eirikbakke I think this would the right time to merge this, as we are early in the cycle and at least my runs in the last few weeks at work looked clean. PRs don't get better when the are just lying around. |
|
@matthiasblaesing Great, merged! Thanks for the useful discussions. |
5 participants
This PR adds scalable SVG versions of a few more NetBeans icons, for Retina/HiDPI screens, originally drawn for the Ultorg project. Some of the existing icons are also copied to new locations that were discovered to be visual duplicates of the same original bitmap images. Part of NETBEANS-2617.
See this HTML page table for a complete summary of icons in the NetBeans codebase as of today and this PR, with identified duplicates and mappings to SVG files.
Details:
The following composite of screenshots shows a few examples of new SVG icons:
This PR adds or touches SVG files only, and includes no code changes. The Adobe Illustrator file that was used to generate the SVG files will be committed in a separate PR (here), along with the script that was used to find duplicates, copy the files to the right locations, and generate the HTML page summary.