Skip to content
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

Use correct font in painter in **Memory Viewer** dialog. #159

Merged

Conversation

cristian64
Copy link
Collaborator

@cristian64 cristian64 commented May 29, 2024

Although the font was set up in the Memory Viewer dialog's main widget, the viewport's painter that actually renders the text was still using the default font.

When a theme (other than System) was used, this resulted in the grid and the text having a mismatched size, which was specially noticeable when the font size was made large or small:

Before After
DME - Memory Viewer (incorrect font size) png DME - Memory Viewer (correct font size)

Bonus changes: initial font size is no longer hardcoded to 15; now it's based on a percentage (+50%) of the size of the default font.

… all platforms.

Previously, the system-defined, fixed-width font was used only for
macOS; now the font is used on Windows and Linux too, as opposed to
using some hardcoded font families that may not be available in the
system.
@dreamsyntax
Copy link
Collaborator

dreamsyntax commented May 30, 2024

I'm not sure what was going on with your before screenshots... I see no change on Windows at least.
Unless I'm misunderstanding and you explicitly made it small for demonstrating the width?

Windows Before:
image

Windows After:
image


Linux Before:
before

Linux After:
after

Ctrl+Mouse Scroll seems to work the same on both builds.

Copy link
Collaborator

@dreamsyntax dreamsyntax left a comment

Choose a reason for hiding this comment

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

The changes seem sane to me, I just don't notice any difference in my tests on both windows and linux (cachyos, kde, wayland)

If it fixes this issue for some other configurations, I don't have a problem with it being merged.
Feel free to merge when you are satisfied.

@cristian64
Copy link
Collaborator Author

I'm not sure what was going on with your before screenshots... I see no change on Windows at least.
Unless I'm misunderstanding and you explicitly made it small for demonstrating the width?

Before taking the screenshots, I had used the mouse wheel to increase the font size. Without this fix (i.e. the screenshot in the Before column), the grid grows to match the font size, but the text is rendered with a smaller font size.

I have now realized that the issue only manifests if the theme is not set to System. Could you confirm you can reproduce the problem with any other theme?

@dreamsyntax
Copy link
Collaborator

I have now realized that the issue only manifests if the theme is not set to System. Could you confirm you can reproduce the problem with any other theme?

Ah yep, thats what it was.
Reproduced on Windows
image

@cristian64 cristian64 force-pushed the adjust_memory_viewer_font_size branch from a1f0e85 to b72785e Compare May 31, 2024 20:04
Although the font was set up in the **Memory Viewer** dialog's main
widget, the viewport's painter that actually renders the text was still
using the default font.

When a theme (other than **System**) was used, this resulted in the grid
and the text having a mismatched size, which was specially noticeable
when the font size was made large or small:

| Before | After |
| --- | --- |
| ![DME - Memory Viewer (incorrect font size) png](https://github.com/aldelaro5/dolphin-memory-engine/assets/1853278/08e2597e-aa6a-4c6e-be5c-1fa46555b340) | ![DME - Memory Viewer (correct font size)](https://github.com/aldelaro5/dolphin-memory-engine/assets/1853278/348ee9f0-c407-4666-b040-caac62a63fb1) |
The initial font size for the **Memory Viewer** dialog is now based on a
percentage of the default font size.
@cristian64 cristian64 force-pushed the adjust_memory_viewer_font_size branch from b72785e to ff6308f Compare May 31, 2024 20:06
@dreamsyntax dreamsyntax merged commit 9ab59e3 into aldelaro5:master May 31, 2024
4 checks passed
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.

2 participants