-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Clean up the Converters implementation #16820
Conversation
} | ||
|
||
winrt::Windows::UI::Text::FontWeight Converters::DoubleToFontWeight(double value) | ||
// Numbers | ||
double Converters::PercentageToPercentageValue(double value) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't this cause the XAML UI to occasionally display something stupid like 99.000000000000000000000000001%
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
making the signature double (double)
, that is
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good point. I'll test this. The only time this should occur is when we convert between float and double, so I'll just find and test all the places where we do that (shouldn't be too many?).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does result in something stupid like that, but not in the way you might have thought:
AppendPercentageSign
has no limitation in the decimal places during stringification which results in these issues. It seems we simply didn't notice because we were lucky.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought the Converters.idl file had a really neat ordering,
and I felt like the .cpp implementation fell short of this.
This PR reorders the functions in the implementation to match the IDL.
It also gets rid of some unnecessary math (int vs. float, clamping)
and removes another use of
std::stringstream
(= bad STL class).