judovana / java-runtime-decompiler

GNU General Public License v3.0
68 stars 14 forks source link

Add image icon #198

Closed mkoncek closed 3 years ago

mkoncek commented 3 years ago

Related: #197

judovana commented 3 years ago

Nice work! I like it! But unluckily, Ondra is mian reviwer:(

Btw, can the png be not sotred, but generated from svg during build?

Also paths in development/image/packaging may be fun

mkoncek commented 3 years ago

I also wanted to have it in svg only, but didn't do enough research... Not sure what you mean with paths.

mkoncek commented 3 years ago

Looks like SVG handling would require an external library. https://xmlgraphics.apache.org/batik/

judovana commented 3 years ago

batik is everywhere :)

judovana commented 3 years ago

The application itself, now of course have in-jar image as icon

The funn (As another PR, by someone else) will be to have this icon properly use das eg launcher icon or so (whichon windows implict binalry launcher and so on.. :D)

mkoncek commented 3 years ago

I tries using batik but i am getting simple NoClassDefFound on various classes from org/w3c/dom. However obtaining a jar seems really weird. Just see for yourself, what am i supposed to import? https://findjar.com/class/org/w3c/dom/svg/SVGDocument.html https://findjar.com/class/org/w3c/dom/smil/ElementTimeControl.html

judovana commented 3 years ago

Nothing? Add it as build time only maven depndece and then use that?

-- Mgr. Jiri Vanek @.***

---------- Původní e-mail ---------- Od: mkoncek @.> Komu: pmikova/java-runtime-decompiler @. github.com> Datum: 18. 10. 2021 18:25:05 Předmět: Re: [pmikova/java-runtime-decompiler] Add image icon (PR #198) "

I tries using batik but i am getting simple NoClassDefFound on various classes from org/w3c/dom. However obtaining a jar seems really weird. Just see for yourself, what am i supposed to import? https://findjar.com/class/org/w3c/dom/svg/SVGDocument.html (https://findjar.com/class/org/w3c/dom/svg/SVGDocument.html) https://findjar.com/class/org/w3c/dom/smil/ElementTimeControl.html (https://findjar.com/class/org/w3c/dom/smil/ElementTimeControl.html)

— You are receiving this because you commented. Reply to this email directly, view it on GitHub (https://github.com/pmikova/java-runtime-decompiler/pull/198#issuecomment-945947502) , or unsubscribe (https://github.com/notifications/unsubscribe-auth/AAWFCS4DIB6TCUZ4FQVKDXLUHRC5TANCNFSM5GGZIR5A) . Triage notifications on the go with GitHub Mobile for iOS (https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675) or Android (https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub) . "

mkoncek commented 3 years ago

I don't know if i used the imports correctly but this way it works. Tell me whal else to change.

AurumTheEnd commented 3 years ago

Also for future ease of reference, here's a direct link to the .png icon before the switch to from-svg generation. Let's see how long the link stays working.

judovana commented 3 years ago

nnn. Pls drop all batik libraries as runtime dependencies, that must not happen. It is build time only one, and use it to convert the source .svg to .png. Maybe instead of batik you can use iame agic or similar?

In runtime, no batik, just png.

I have to contradict Prokop Tunel - I like the icon, I like the different sizes of dashes. Really I do. I like the colors. The whole icon represents JRD really well.

Do you mind to add small white JRD text to the middle of the icon?

mkoncek commented 3 years ago

Ok, i understand now, you want to create a png during the build and use it directly at runtime. I think that can be done with imagemagick, i believe that is just about everywhere, using maven-exec-plugin. Although there would be a problem with this approach in CI or when building on windows...?

mkoncek commented 3 years ago

First of all, thank you for the effort. As arguably the main developer of JRD for the past 3 months, could you help me see some of the symbolism you sought after in the icon?

* What do the colors represent?

No reason, i think they look nice

* What does the cycle represent?

The cycle of download == decompiling / upload == recompiling. Although my point of view may be biased. I think this is the main usage of JRD?

* Why is one arrow dashed?

It symbolizes that the recompilation is imperfect, ad-hoc and may fail.

mkoncek commented 3 years ago

Also nit picking, the top-most dash is a different size to the others: image

I know, i was calibrating the curved line by the end, not by the beginning, but you are right, it will look better if the dashes are the same size.

AurumTheEnd commented 3 years ago

Although there would be a problem with this approach in CI or when building on windows...?

We'll have to test that bridge when we get to it.

mkoncek commented 3 years ago

With dashes of the same size main-icon

With "JRD" (arrows rotated a bit) main-icon-jrd

Personally I don't llike the text much.

mkoncek commented 3 years ago

1) There is a possible NullPointerException if the resource does not exist (will be fixed after we decide how to generate png) 2) Should i use a custom <profile> in exec-maven-plugin or just somehow try to call convert and ignore if it fails? 3) Should the generated png be placed into .../src/resources/icons? I believe in maven usually sources are not supposed to be changed which is why there are directories like target/generated-sources and so on, but i don't know if there is anything like generated-resources.

judovana commented 3 years ago

I like the version with thext - can be smaller (and so arrows can remain unroated) a bit more. Prokop Tunel may have vote to decide. But Ilike botha.. all three ;)

judovana commented 3 years ago
1. There is a possible `NullPointerException` if the resource does not exist (will be fixed after we decide how to generate png)

2. Should i use a custom `<profile>` in exec-maven-plugin or just somehow try to call `convert` and ignore if it fails?

3. Should the generated png be placed into `.../src/resources/icons`? I believe in maven usually sources are not supposed to be changed which is why there are directories like `target/generated-sources` and so on, but i don't know if there is anything like `generated-resources`.

There is weird middle path - to ahve commted both svg and png, and have special maven cycle to generate png. But in ths case it is moreover the same as simply commit it. And later edit it and convert and commit both.

You are right the NPE is thretening here, but may be workarounded in loading - if icon do not exists - then well, ntohing happens (you can make this check by going through:\

 URL u = getResurce(string)..if u ==null, then resource do not exists

So with this,m the development version would have defailt system jdk version - unless a custom target to generate the image is run. (the resource must be in gitignore then) and the released image will f couorse have the icon. As we are handling the rpm distribution, we can run that special target in spec. WDYT?

tbh - it sounds a bti overengineerd.

mkoncek commented 3 years ago

1) I got lost in your monologue but pushing the png in the repo may be a good solution. 2) What about resource licensing? It may be a bit problematic when packaging in Fedora, I put a CC0 on the svg file, but afaik if you don't specify anything, the content is licensed by the project license? 3) Keep in mind that icon may be very small and the JRD would be barely visible.

judovana commented 3 years ago

put the file under mit !

-- Mgr. Jiri Vanek @.***

---------- Původní e-mail ---------- Od: mkoncek @.> Komu: pmikova/java-runtime-decompiler @. github.com> Datum: 19. 10. 2021 16:50:59 Předmět: Re: [pmikova/java-runtime-decompiler] Add image icon (PR #198) "

  1. I got lost in your monologue but pushing the png in the repo may be a good solution.
  2. What about resource licensing? It may be a bit problematic when packaging in Fedora, I put a CC0 on the svg file, but afaik if you don't specify anything, the content is licensed by the project license?
  3. Keep in mind that icon may be very small and the JRD would be barely visible.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub (https://github.com/pmikova/java-runtime-decompiler/pull/198#issuecomment-946803752) , or unsubscribe (https://github.com/notifications/unsubscribe-auth/AAWFCSZU6MDFZNCAI3ZW3JLUHWAU7ANCNFSM5GGZIR5A) . Triage notifications on the go with GitHub Mobile for iOS (https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675) or Android (https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub) . "

judovana commented 3 years ago

sorry. Long story short a) put the file under mit b) commit svg and png c) made the loading code NPE proof.

ANything else can be done later

mkoncek commented 3 years ago

Clear, but there is a little problem. Adding an XML comment with the license text into the svg will work, but editing the file and saving it will overwrite it. No one will keep this in mind and simply saving it will remove the copyright statement. Therefore I am in favor of not specifying any copyright statement at all and take it simply as the project contribution governed by whatever is the project license.

judovana commented 3 years ago

ok!

-- Mgr. Jiri Vanek @.***

---------- Původní e-mail ---------- Od: mkoncek @.> Komu: pmikova/java-runtime-decompiler @. github.com> Datum: 20. 10. 2021 10:23:02 Předmět: Re: [pmikova/java-runtime-decompiler] Add image icon (PR #198) "

Clear, but there is a little problem. Adding an XML comment with the license text into the svg will work, but editing the file and saving it will overwrite it. No one will keep this in mind and simply saving it will remove the copyright statement. Therefore I am in favor of not specifying any copyright statement at all and take it simply as the project contribution governed by whatever is the project license.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub (https://github.com/pmikova/java-runtime-decompiler/pull/198#issuecomment-947439480) , or unsubscribe (https://github.com/notifications/unsubscribe-auth/AAWFCS7UBT4YQQFPW3PXXYTUHZ357ANCNFSM5GGZIR5A) . Triage notifications on the go with GitHub Mobile for iOS (https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675) or Android (https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub) . "

judovana commented 3 years ago

thanx!

AurumTheEnd commented 3 years ago

@mkoncek Hi, please note that you don't use your GitHub email in your repo's git config and thus in your commits for this repo, so you unfortunately aren't yet tracked as a contributor to the project.

image