Opened 8 months ago

Closed 6 months ago

#18590 closed defect (fixed)

Font description string Get and Set bug in wx3.1.3 on Windows 32bit build

Reported by: ollydbg Owned by: Vadim Zeitlin <vadim@…>
Priority: normal Milestone: 3.1.4
Component: GUI-all Version: 3.1.3
Keywords: Cc: paulclinger@…
Blocked By: Blocking:
Patch: no

Description

Hi, I see an alert when I load the font description string.

My environment is: OS: Windows7, Compiler: mingw-w64's 32bit gcc 8.1, wxWidgets 3.1.3

To reproduce this error, just build the samples/font

Run the sample, first click menu->Select->font, and select a font, for example, I select "Terminus".
The font info string will shown in the dialog.

Second, I click the Menu->Font->set native font description, this pops up an alert message says the description string is wrong, see image show as attachment.

Any ideas?

BTW: This issue is from my reported bug in Code::Blocks, see here: http://forums.codeblocks.org/index.php/topic,23567.0.html, and I finally see this is a wxWidgets bug.

Thanks.
Asmwarrior

Attachments (1)

font-sample-error.png download (62.1 KB) - added by ollydbg 8 months ago.

Download all attachments as: .zip

Change History (6)

Changed 8 months ago by ollydbg

comment:1 Changed 8 months ago by MaartenB

I see it too in master on Win10 with MSVC 2019 x64.
Looks like the 13.8 point size in the font string is the cause. It has different double and float values and fails on https://github.com/wxWidgets/wxWidgets/blob/master/src/msw/font.cpp#L696

Oh, I see vadz has the same conclusion in wx-users

Last edited 8 months ago by MaartenB (previous) (diff)

comment:2 Changed 8 months ago by vadz

  • Milestone set to 3.1.4
  • Status changed from new to confirmed

Yes, it's due to precision loss when converting from double to float. But it's actually worse than that, there can be not only precision loss, but annoying "precision gain" when serializing and deserializing floats to strings and back, i.e. we don't want to save the font of size 9.1 and restore it as size 9.0999993 or something like that, as it will look ugly if the program shows the font size anywhere in the UI.

The sad thing is that I'm perfectly aware of this problem, but somehow failed to think about it when writing this code. The solution is to use scaling and serializing integers only, i.e. save 9.1 as 9100 (because nobody should need more than 3 digits of font size precision, right?). Unfortunately this means changing the version again and I'd like to avoid it, so I'm thinking about saving 9.1 as "9.100", i.e. inserting the dot artificially into what is basically int(size*1000). This should allow making this work correctly without the version change, AFAICS, so now I just need to find the time to do it...

comment:3 Changed 8 months ago by paulclinger

  • Cc paulclinger@… added

annoying "precision gain" when serializing and deserializing floats to strings and back, i.e. we don't want to save the font of size 9.1 and restore it as size 9.0999993 or something like that, as it will look ugly if the program shows the font size anywhere in the UI.

Maybe this can be solved by enforcing specific number of significant digits with ".03f" or similar? The comparison can be done with two (converted) strings using the same format instead of two numbers, which would solve the precision issue. This method is probably used infrequently enough to create performance issues, but I'm not sure what else may be affected.

comment:4 Changed 6 months ago by vadz

Finally I think that perhaps we don't need to do anything complicated here. The max difference between the double corresponding to the string representation of a float and the original float is of the order of 1e-38, i.e. not really significant. So I think the simple approach of this PR should be enough.

Does anybody have any problems with doing it like this?

comment:5 Changed 6 months ago by Vadim Zeitlin <vadim@…>

  • Owner set to Vadim Zeitlin <vadim@…>
  • Resolution set to fixed
  • Status changed from confirmed to closed

In a73194f6/git-wxWidgets:

Allow parsing all fractional sizes in wxFont descriptions

Remove the check that the size representation was the same as float and
as double, which was supposed to catch various edge cases (NaNs, huge
numbers etc) but actually caught plenty of perfectly valid font sizes
such as 13.8 that simply lost precision when converting from double to
float.

Just check that the size is positive and less than FLT_MAX to avoid
using values that really don't make sense as font sizes.

Also add a unit test checking that using fractional font sizes in
description string works as expected.

Closes #18590.

Closes https://github.com/wxWidgets/wxWidgets/pull/1707

Note: See TracTickets for help on using tickets.