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

Qt/Debugger: Memory View font based sizing, row coloring #7698

Open
wants to merge 1 commit into
base: master
from

Conversation

3 participants
@TryTwo
Copy link
Contributor

TryTwo commented Jan 12, 2019

Allows more rows to fit on screen.
Added alternating row colors. I can remove it if disliked.
Added a mark to always indicate the middle (address focused) row. I didn't want to mark the whole row, so the BP box is just darker grey.

@BhaaLseN
Copy link
Member

BhaaLseN left a comment

Could you add a screenshot so we can see what it looks like?

Show resolved Hide resolved Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp Outdated
@TryTwo

This comment has been minimized.

Copy link
Contributor

TryTwo commented Jan 12, 2019

memoryview
Left image uses the 2-column mode PR #7678.

@TryTwo TryTwo force-pushed the TryTwo:Debugger_UI_MemoryView_Font_Based_Sizing branch from 803366e to d37bae7 Jan 12, 2019

@TryTwo

This comment has been minimized.

Copy link
Contributor

TryTwo commented Jan 12, 2019

Oops, forgot to change the Update function using a constant row height. Also fonth -> rowh.

@TryTwo TryTwo force-pushed the TryTwo:Debugger_UI_MemoryView_Font_Based_Sizing branch from d37bae7 to 23f0f40 Jan 12, 2019

@TryTwo

This comment has been minimized.

Copy link
Contributor

TryTwo commented Jan 12, 2019

And forgot to set variable BP column/icon size. Should be all done now.

Resources::GetScaledThemeIcon("debugger_breakpoint").pixmap(QSize(24, 24)));
bp_item->setData(
Qt::DecorationRole,
Resources::GetScaledThemeIcon("debugger_breakpoint").pixmap(QSize(rowh, rowh)));

This comment has been minimized.

@BhaaLseN

BhaaLseN Jan 12, 2019

Member

Is it a good idea to scale this icon?

This comment has been minimized.

@TryTwo

TryTwo Jan 12, 2019

Contributor

I assumed GetScaledThemeIcon meant it was originally being scaled as well. 24 was the old constant row height. I don't see any problems/bugs from scaling it to rowh. Just prevents the tighter row height from chopping off the top.

@TryTwo TryTwo force-pushed the TryTwo:Debugger_UI_MemoryView_Font_Based_Sizing branch from 23f0f40 to 948fc89 Jan 15, 2019

@TryTwo

This comment has been minimized.

Copy link
Contributor

TryTwo commented Jan 15, 2019

I noticed this doesn't work as well as code view's font based sizing, because it doesn't re-calibrate on font change/signal. I'll have to update it.

@TryTwo TryTwo force-pushed the TryTwo:Debugger_UI_MemoryView_Font_Based_Sizing branch from 948fc89 to 1c664a2 Jan 15, 2019

@TryTwo

This comment has been minimized.

Copy link
Contributor

TryTwo commented Jan 15, 2019

Moved the verticalheader stuff into Update and tweaked it.
Run Update on font change.
Added horizontalHeader()->setMinimumSectionSize to let BP column be smaller.

verticalHeader()->setDefaultSectionSize appears to do what setRowHeight was doing just fine, so removed setRowHeight..

Better supports small font sizes now:
memoryviewsmallfont

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment