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

Remove if-False code #2235

Closed
wants to merge 1 commit into from

Conversation

gasull
Copy link

@gasull gasull commented Apr 6, 2021

This code was disabled over a year and a half ago and isn't needed anymore.

Related issue: #2213


This change is Reviewable

@EchterAgo
Copy link

We changed this when we switched to Freetype on Windows, which made the render good. Due to other issues we later made this optional and default to off, but never switched back to (S). This should probably be conditional so that it uses (S) only if the platform is Windows and Freetype is disabled.

@EchterAgo
Copy link

FYI, this is how the ⓢ looks on Windows with native font rendering:

image

Really bad, and that is on a High-DPI 4K screen. Somehow Windows does not do Cleartype for that character at all.

@cculianu
Copy link
Collaborator

cculianu commented Apr 6, 2021

Yeah we probably want to both detect freetype and window here somehow -- how would one detect freetype? Would that be a detection that can be made once at "module import time"?

@EchterAgo
Copy link

I would just use the windows_qt_use_freetype setting and test for sys.platform in ('win32', 'cygwin')

@gasull
Copy link
Author

gasull commented Apr 7, 2021

I would just use the windows_qt_use_freetype setting and test for sys.platform in ('win32', 'cygwin')

So if we're on Windows and FreeType is detected, we should still use SCHNORR_SIGIL = "ⓢ". Correct?

And if we're on Windows but FreeType isn't detected, should use SCHNORR_SIGIL = "(S)". Am I right?

@EchterAgo
Copy link

So if we're on Windows and FreeType is detected, we should still use SCHNORR_SIGIL = "ⓢ". Correct?

Yes

And if we're on Windows but FreeType isn't detected, should use SCHNORR_SIGIL = "(S)". Am I right?

Yes

@gasull
Copy link
Author

gasull commented Apr 8, 2021

I'm closing this PR in favor of a new one: #2236

@gasull gasull closed this Apr 8, 2021
@gasull gasull deleted the bch-rm-unsatisfiable-if branch April 8, 2021 02:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants