Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Brackets [] wrongly displayed in Intellij when focused (bold) #129

Closed
hrach opened this issue Oct 15, 2015 · 78 comments
Closed

Brackets [] wrongly displayed in Intellij when focused (bold) #129

hrach opened this issue Oct 15, 2015 · 78 comments

Comments

@hrach
Copy link

hrach commented Oct 15, 2015

When I put cursor to brackets, they get bold and the bottom line is not visible at all.
font-intellij

Windows, Hack-v2_015-ttf, size 12, changing line-spacing does not help.

@jorgheymans
Copy link
Contributor

FWIW i do not see this issue, lntellij 14.1.5 - hack v15

image

@hrach
Copy link
Author

hrach commented Oct 15, 2015

Any idea how to debug it? PhpStorm, PS 141.x

@jorgheymans
Copy link
Contributor

check #45 maybe, i had some caching issues when i upgraded the font from the previous version that had this behaviour.

@chrissimpkins
Copy link
Member

Try issue report #100 where a similar problem was identified. They reported an issue on the JetBrains issue tracker. It sounds like this was fixed as of the v2.015 release.

@hrach
Copy link
Author

hrach commented Oct 16, 2015

  • I have fresh new laptop, so, from the beginning I am using hack v15.
  • though, I tried to uninstall the font, invalidated caches in PS, nothing helped
  • I tried to upgrade to PS 9.5 EAP, didn't help.

Important note: previously mentioned bugs seem to be caused by current line highlighting, which overlay-ed the bottom parts of letters on the previous line. My issue seems to be buggy in every case when the bracket is bolded.

@hrach
Copy link
Author

hrach commented Oct 16, 2015

But as mentioned in the other bugs, changing font-size to bigger don't help, however, 11 works ok.

@hrach
Copy link
Author

hrach commented Nov 10, 2015

PhpStorm 10. v18, bug is still present.

@chrissimpkins
Copy link
Member

Thank you

@jorgheymans
Copy link
Contributor

just reporting in that this is still present in 2.020 release, i have created a new ticket on the intellj side https://youtrack.jetbrains.com/issue/IDEA-155229

@chrissimpkins
Copy link
Member

@jorgheymans mind trying the 25% line spacing script (see top of readme) to see if it addresses this in IntelliJ?

@jorgheymans
Copy link
Contributor

i'm on windows only here sorry , @hrach maybe ?

@chrissimpkins
Copy link
Member

happy to build them for you if that is the problem.

@jorgheymans
Copy link
Contributor

sure i can try out whatever zip you make available no problem.

@chrissimpkins
Copy link
Member

chrissimpkins commented Apr 26, 2016

Give these a try. This slightly increases the default line spacing across all variants.

hack-tests.zip

Do you know how to clear the Windows font cache and confirm that there are not duplicated font files with font upgrades?

@jorgheymans
Copy link
Contributor

well i remove them from the font dir first, then drop the new fonts in the windows font dir is that sufficient? also restarting intellij ofcourse.

In any case it did not solve the issue it seems.

@hrach
Copy link
Author

hrach commented Apr 26, 2016

I did the same as @jorgheymans with the same result. (Windows 10)

@chrissimpkins
Copy link
Member

on my phone at the moment. let me track down @texhex instructions and link them for you later today.

@texhex is there a simple way to script this for testers on Windows?

@CrazyCoder
Copy link

CrazyCoder commented Apr 26, 2016

Not IntelliJ IDEA specific, NetBeans has the same problem with bold Hack font:

netbeans64_2016-04-26_13-28-41

Also note incorrect glyphs for bold italic: https://bugs.openjdk.java.net/browse/JDK-8078382.

@chrissimpkins
Copy link
Member

chrissimpkins commented Apr 26, 2016

Also note incorrect glyphs for bold italic

@CrazyCoder This is sounding like a caching / font duplication issue. We saw this when Windows users' systems did not clear the previously installed versions (this is the default unfortunately). Are you willing to try our new Windows installer to see if it addresses this?

cc: @texhex

@chrissimpkins
Copy link
Member

@CrazyCoder It looks like there may be clipping of the parentheses glyphs in those images as well. Is that the case?

@CrazyCoder
Copy link

Hmm, this was the first time I installed Hack font on this system, however I tried the installer and it has fixed the problem. Weird.

idea64_2016-04-26_17-34-31

@chrissimpkins
Copy link
Member

@CrazyCoder Windows makes our head ache :) The current Windows installer contains v2.019. v2.020 should be available sometime in the next few days. I would recommend the 2.020 update to Win users. We addressed the hints in some important glyphs (including the zero) that should make a difference on the platform.

@chrissimpkins
Copy link
Member

@hrach @jorgheymans want to give the installer a try? it looks like it fixed the bracket problem for @CrazyCoder

@chrissimpkins
Copy link
Member

@CrazyCoder can you let me know how you performed the initial install?

@texhex
Copy link
Member

texhex commented Apr 26, 2016

@hrach @jorgheymans @CrazyCoder

The Hack Windows installer has been updated to install Hack v2.020. Please give it a try.

CC: @chrissimpkins

@texhex
Copy link
Member

texhex commented Apr 26, 2016

@chrissimpkins

is there a simple way to script this for testers on Windows?

Sorry, but I don't understand what should be scripted? If there is this special character issue?

@CrazyCoder
Copy link

can you let me know how you performed the initial install?

Right click on .TTF files, Install.

@chrissimpkins
Copy link
Member

@texhex Wondering if there is a way to provide the functionality of the install tool without the need to compile the installer for those who help test specific issues in the fonts. This wouldn't be used for distribution, but to provide assurance that the problems that we identified (and are addressed with the installer) are not responsible for the issue persistence when testers attempt fixes through new installs.

@texhex
Copy link
Member

texhex commented Apr 28, 2016

Wow, thanks! This is very useful and much appreciated. And it will be very useful to easily detect rending issues, just as you proposed.

Under which license do you plan to release this source? Would MIT be OK for you? I'm asking because I think it would make sense to push this code to a repository so it is not "lost" in this issue comment.

@chrissimpkins Please have a look at the picture, does it help to identify any hinting issues? Also, please let me know if you agree with moving the code to a separate repository.

@jorgheymans
Copy link
Contributor

@texhex it's just an adapted version of the test class that was used in the openjdk ticket, you can use it or modify it as you want. I would be happy to improve it still if needed. Good point about moving it outside of the ticket somewhere, probably could be made more visible.

Now, what I noticed just now looking at the picture is that the brackets are only cut off in the LCD HRGB and LCD HGBR aliasing mode, other modes just work fine. Something to do with subpixel rendering perhaps. The rendering hints are described here https://docs.oracle.com/javase/7/docs/api/java/awt/RenderingHints.html in case it helps..

@texhex
Copy link
Member

texhex commented Apr 28, 2016

Yes, LCD HRGB and LCD HGBR screw up the lower part of [] when using Bold, while DEFAULT, LCD VBGR and LCD VRGB seems to mess up [ when using Normal (Regular).

VALUE_TEXT_ANTIALIAS_GASP and "Antialiased Text Mode" seem to be fine always.

Could you please explain what is this T2K scaler is about?

When I understood it correctly (Source#1 Source#2), the JDK now uses the Freetype font scaler which seems to have problems with some fonts on Windows when using native scaling, while the T2K scaler is the older font scaler using by the Sun JDK which doesn't had this issue?

@texhex
Copy link
Member

texhex commented Apr 28, 2016

@jorgheymans

I would be happy to improve it still if needed.

Awesome! Let's see what the godfather of Hack says to all of this.

@chrissimpkins
Copy link
Member

I'm gonna make them an offer they can't refuse...

@texhex
Copy link
Member

texhex commented Apr 28, 2016

I'm gonna make them an offer they can't refuse...

I have the slight feeling I will regret calling you godfather of Hack...

@chrissimpkins
Copy link
Member

chrissimpkins commented Apr 28, 2016

I have the slight feeling I will regret calling you godfather of Hack...

Ha!

Re: How to address this in documentation

@texhex @jorgheymans I read the Java issue report last evening and it appears that they closed this as fixed in the patched JDK if I understood it correctly. Do you happen to know if it is possible to define a different JDK in the JetBrains editors? If so, perhaps we provide users with this workaround in the docs.

Would either of you be interested in submitting a PR to update our README with the information that Win + JetBrains users need based upon your thoughts?

Re: the new Java tool

@jorgheymans Jorg, would you be willing to push this tool (or some variant of it that @texhex and you feel will best help to address our Java woes) to a repository and license it so that we can either point users to the tool in order to troubleshoot similar problems or perform this testing ourselves?

Further Info

Perhaps we could dig in to the Java documentation on these glyph rendering classes too. The drawString() method on the Graphics2D class and the getAscent() method on the FontMetrics class may yield something that I can work with. I would be very interested to know what vertical metric the getAscent() method is retrieving from the font because there are multiple OpenType table values (that can differ within the same font and differ in approach between fonts) that are options for use. If this is the property that is being used to determine line height and between line vertical spacing we might be on to a solution via the Hack metrics. It remains to be seen whether we can modify this value to address this situation, but at least we know.

@chrissimpkins
Copy link
Member

@jorgheymans and thank you for this fantastic Java tool by the way! Much appreciated! I am learning about Java font rendering from your code.

@chrissimpkins
Copy link
Member

Addendum to #129 (comment):

Is it possible, and would it be worthwhile, to add a header in this reporting tool that displays what default antialiasing settings the user's system is using or is this uniform across Windows releases?

@jorgheymans
Copy link
Contributor

@texhex your understanding about the T2K scaler seems correct.

@chrissimpkins you can choose whatever license you want for that class. Should it not stay within the hack repository somewhere ? Otherwise just let me know where to push it to.

About the default antialiasing settings: if there are no rendering hints specified for the text antialiasing then it falls back to the setting specified for all antialiasing, the setting for this is implementation dependent . It would take a decent amount of digging in the jdk sources to figure out what these settings are, i do not think they are meant to be exposed on the jdk level.

Windows and the jdk are two different things, there is no such thing as default settings on the jdk level because every java program is expected to set these settings as they see fit, at least that is my understanding.

In terms of switching jdk, this is doable for the masses as long as its a released version. The jdk used for starting intellij does not have to be the one used for compiling the project . We could add all this in the README. I will have a look to send a PR the next few days to explain all this in the README somewhere.

@chrissimpkins
Copy link
Member

every java program is expected to set these settings as they see fit

I see. Thanks

In terms of switching jdk, this is doable for the masses as long as its a released version

This is a potential solution then, albeit a very onerous one to address your typeface display...

I will have a look to send a PR the next few days to explain all this in the README somewhere.

That would be great if you are willing to do it. If not, please simply let me know what you feel users need to know to work around this issue. Our README is very long, so the more concise the better, particularly since this only applies to a subset of users.

Should it not stay within the hack repository somewhere ?

I would prefer to expose this in its own repository so that we avoid cluttering the issue report threads here with tool issues. I would be happy to host it in our Source Foundry organization which is where we keep our other project tools, or feel free to push to your own personal account and we would be happy to send people your way when this tool is needed.

Also, can you confirm that this block:

            if (args.length == 1) {
                DEFAULT_TEST_STRING = args[0];
            }

modifies the default constant string defined at the top of the file with the first argument to the executable? This would definitely be a desired feature so that we can test other glyphs that prove problematic.

We could even consider making this available as a general tool for typeface developers and provide a way for them to specify the typeface to be tested on the CL. Some thoughts. We can start here for now.

Let me know how you would like to approach the repository. Thanks again for all of your efforts. Will acknowledge these contributions on our contributors list.

@jorgheymans
Copy link
Contributor

ok for me to host that class somewhere in the source foundry repo.

Indeed, the test class allows for a string to be passed on the command line :

"C:\Program Files\Java\jdk1.8.0_91\bin\java" -jar hackui.zip "[#] __ [#]"

will render the canvas using "[#] __ [#]" (without the quotes).

i was thinking as an improvement it would also read text from a file, for those characters that are difficult to pass on the command line. Also capturing the canvas automatically into a jpg would allow for easy regression testing.

@chrissimpkins
Copy link
Member

@jorgheymans sure sounds great. what would you like to name it?

Like the file read idea. All sounds good!

@jorgheymans
Copy link
Contributor

how about "java glyph tester" -> "a tool for testing the JDK font rendering behaviour"

@chrissimpkins
Copy link
Member

@chrissimpkins
Copy link
Member

capturing the canvas to a jpg...

Through this tool or an external tool? Is there a simple way to do this in Java?

@jorgheymans
Copy link
Contributor

sure it's supposed to be only a couple of lines going through ImageIO , i will have a look at it.

@chrissimpkins
Copy link
Member

Sounds good. This will be a helpful tool. Thanks Jorg!

@chrissimpkins
Copy link
Member

@jorgheymans you should have write access on the Java Glyph Tester repository now. Let me know if you have any issues with it or if there is anything that I can do to help.

cc: @texhex

@texhex
Copy link
Member

texhex commented May 2, 2016

@jorgheymans Thanks for all the time and effort you put into this Jorg, I'm very sure this tool will help us a lot to get the Freescaler under control or at least prove that it's a Windows/Java issue and not an issue of the font.

@jorgheymans
Copy link
Contributor

Access is working fine, just did initial commit.

@texhex no worries 👍

@chrissimpkins
Copy link
Member

@jorgheymans great! let us know what, if anything, we can do to help.

@chrissimpkins
Copy link
Member

To try to finalize this issue, are we in agreement about a workaround here?

@jorgheymans
Copy link
Contributor

yes, i will send a PR this week sometime to the README explaining what is going on and how to workaround. (but it seems there is a PR pending not sure i have to wait for this one to be applied first). How about i put it in a README-intellij.md , catches the attention of those using the tool and others can just ignore it ?

@chrissimpkins
Copy link
Member

No rush on this. We can wait until the revisions on the README are complete. Just wondering what the approach will be.

@jorgheymans
Copy link
Contributor

It looks as if this issue is fixed in latest intellij 2017.1 with jdk8_121, hurray !

@chrissimpkins
Copy link
Member

Closing as fixed based upon comment in #129 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

5 participants