On 08.12.19 14:57, Fred Kiefer wrote:
Am 08.12.2019 um 13:40 schrieb Sergii Stoian <[email protected]>:
On 8 Dec 2019, at 10:12, Fred Kiefer <[email protected]> wrote:
Am 08.12.2019 um 03:07 schrieb Sergii Stoian <[email protected]>:
I suppose it’s not direct art problem. Perhaps art should be adopted to
NSStringDrawing changed caching, I don’t know until I understand string drawing
change. What was the idea of that rewrite? Commit message doesn’t explain why
that change was made. For me, It doesn’t look as “cleanup”. That’s why I asked
you as author of that change.
What I changed back then was to have the scratch item that gets used for the
current drawing to be of the same structure as the cached items. The scratch
item now even gets stored in the same array as the cached one, so the
initialisation is identical. This really simplified the code and helped me
understand it better to allow for further restructuring. Try to understand the
code before and after that change and you may see the difference. The code is
still complicated but at least the data structures are cleaner.
Thanks, now I understand your intention.
I also switched to using explicit types, for example for boolean values. But I
may have made a mistake there. On line 501 (and again on line 674) I replaced
the value 1 with NO and this looks wrong. As this stands for using screen fonts
it would affect the art backend, being the one where we actually support screen
fonts.
I cannot remember what was the reasoning behind that change. The difference
between the actual drawing code and this bounds computation is that during the
drawing we have the transformation to the screen and are able to determine
whether this is close to identity, in which case we use screen fonts. The
bounds computation may happen at any time, so we don’t know if screen fonts
should be used. Maybe the art backend relies on these being used anyway? You
could give it a try and use YES here instead. The lines in the current code are
513 and 606
That’s it, thank you! I’ve changed lines you’ve mentioned and problem gone.
What’s the point to change YES to NO? Was it intentional (some cairo backend
specifics)?
I’ll prepare PR once I setup environment and compile master branch to test
changes.
As I wrote, I cannot remember whether I did that on purpose or not. Both is
possible. Both values are wrong, in some sense as I explained. That is why I
might have changed them. But it is more likely that I just made a mistake here.
Otherwise I would have documented the „improvement“ :-(
I've created a PR #36. It's plain simple revert NO to YES at lines
you've mentioned.
However, another problem exists at master branch. One of the effects is
shown on my WorkspaceMenu+HelpPanel screenshot: selection of index item
shifted vertically. This problem appeared on 0.25.1 branch before NO/YES
change, but got fixed after change. It's not the case with master.
I suppose something wrong with empty string height detection
(LayoutManager?). I need to investigate further.
Sergii
