Skip to content

added option to scale tooltip rendering#151

Merged
majcosta merged 5 commits into
1dot13:masterfrom
NorthFury:improve-tooltip-rendering
May 28, 2023
Merged

added option to scale tooltip rendering#151
majcosta merged 5 commits into
1dot13:masterfrom
NorthFury:improve-tooltip-rendering

Conversation

@NorthFury
Copy link
Copy Markdown
Contributor

The old rendering is unchanged in function, despite the heavy refactoring.
Some existing tooltip rendering issues are fixed now, for example, the bold character is no longer considered when computing the tooltip width.
I removed DEC_INTERNAL_LEADING since WinFont is not supposed to be used except for the Chinese build. It also fixes the font alignment if it is enabled for other builds.

The fast help text is parsed multiple times. This resulted in some duplication of a good chunk of code. Doing this cleanly would mean to parse it when updating FastHelpText. The parsing result would be a <list<list<WeightedText>> that would be saved besides FastHelpText. This represents a list of lines. Each line would have parts of text that is continuously bold or regular font. WeightedText would be a struct with bool isBold and STR16 text properties.

Known Issue:

  • The bold and the regular font rendering doesn't align properly in regards to height. Rounding is done when selecting the font and I think this results in this inconsistent height alignment. A potential solution may be to provide fixed scaling factors with predefined LOGFONT settings. The issue itself is not large as we are speaking of 1-3 pixels misalignment.

Copy link
Copy Markdown
Contributor

@majcosta majcosta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, next time try to separate the meaty changes, whitespace changes (not the newline additions, that's the .editorconfig file's doing, not you) and dead code removal into their own commits, it makes reviewing the PR much easier.

If the actual code changes can themselves be split up into smaller logical commits, even better.

I tested English and German and it works fine, but Chinese tooltips are broken with TOOLTIP_SCALE_FACTOR > 100

Comment thread Strategic/Map Screen Interface.cpp
Comment thread Standard Gaming Platform/WinFont.h
Comment thread Standard Gaming Platform/WinFont.cpp Outdated
Comment thread Standard Gaming Platform/mousesystem.cpp Outdated
Comment thread Standard Gaming Platform/mousesystem.cpp Outdated
Comment thread Tactical/SoldierTooltips.cpp Outdated
@NorthFury
Copy link
Copy Markdown
Contributor Author

NorthFury commented May 24, 2023

Please, next time try to separate the meaty changes, whitespace changes (not the newline additions, that's the .editorconfig file's doing, not you) and dead code removal into their own commits, it makes reviewing the PR much easier.

If the actual code changes can themselves be split up into smaller logical commits, even better.

I'm only formatting the parts of the code that I'm touching and in the case of changing most of the function code, I might format the full function.
I should have mentioned that most functions in the mousesystem.cpp file were written from scratch and it's best to review it function by function instead of line by line.
I will try to be more mindful of the review process for the next changes.

I tested English and German and it works fine, but Chinese tooltips are broken with TOOLTIP_SCALE_FACTOR > 100

I didn't try it out with the Chinese build. I hope that changing the build target will suffice to be able to test it on my side.

@majcosta
Copy link
Copy Markdown
Contributor

I didn't try it out with the Chinese build. I hope that changing the build target will suffice to be able to test it on my side.

You'll need a Chinese gamedir as well. It's the same process as running English 1.13, with the extra step of extracting https://github.com/1dot13/gamedir-languages/tree/master/Chinese_Version on top of it.

Copy link
Copy Markdown
Contributor

@majcosta majcosta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool, Chinese works now. thanks!

@majcosta majcosta merged commit 1c82efb into 1dot13:master May 28, 2023
@NorthFury NorthFury deleted the improve-tooltip-rendering branch May 28, 2023 15:58
kitty624 added a commit to kitty624/1dot13-gamedir that referenced this pull request Feb 9, 2024
the source pr 1dot13/source#151
made it possible to scale up tooltips

added the setting and description to ja2.ini to allow to make use of it
kitty624 added a commit to 1dot13/gamedir that referenced this pull request Feb 9, 2024
Update to ja2.ini for scaling tooltips

he source pr 1dot13/source#151 made it possible to scale up tooltips

added the setting and description to ja2.ini to allow to make use of it
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.

3 participants