Conversation
| EditChord::addChordParentheses(chord, { note }); | ||
| } | ||
|
|
||
| static constexpr int OLD_DPI = 360; |
There was a problem hiding this comment.
This should probably be called PRE_470_DPI
| { | ||
| // The frame radius used to be expressed in raster units and divided by 2 at drawing. Since 4.7 it is expressed in spatium. | ||
| static constexpr int OLD_DPI = 360; | ||
| return Spatium(frameRadius * (DPI / OLD_DPI) / DefaultStyle::baseStyle().value(Sid::spatium).toDouble()) / 2; |
There was a problem hiding this comment.
Not your bug, but you should probably replace:
(DPI / OLD_DPI)With:
(static_cast<double>(DPI) / OLD_DPI)This ensures the division is computed as a double (i.e. floating-point) rather than truncated to int. (C++ arithmetic uses the higher-precision type, so only one operand needs to be double for the result to be double.)
This would also fix it, but it's less explicit, so probably best avoided:
(frameRadius * DPI) / OLD_DPIOf course, if DPI and OLD_DPI divide cleanly then it makes no difference, at least until one of them is changed in the future.
| { | ||
| if (image->size().isNull() && image->score()->mscVersion() < 470) { | ||
| image->init(); | ||
| image->setSize(image->size() * (DPI / OLD_DPI)); |
There was a problem hiding this comment.
Potentially the same bug as above, though I'm not sure whether image->size() is an integer or floating-point type, and what the input to setSize() needs to be.
But even if these are all integers, there will be a rounding error unless you at least do:
image->setSize((image->size() * DPI) / OLD_DPI); // Multiply first to avoid rounding error.There was a problem hiding this comment.
DPI is actually already a double so this is not a problem, but you're right that OLD_DPI (now called PRE_470_DPI) should be a double too, for clarity.
mike-spa
force-pushed
the
fixImageSizePre470
branch
from
8a4ab49 to
80db03a
Compare
3 participants
Resolves: #32387