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

Cursor not behaving correctly in memory view editor while chaging variable value on Hi-DPI #645

Merged
merged 1 commit into from
Dec 27, 2023

Conversation

mshahzadiftikhar
Copy link
Contributor

@mshahzadiftikhar mshahzadiftikhar commented Dec 14, 2023

Description
Changing value of a hex digit in a cell in Memory View is not working correctly on higher resolution displays.

Problem
GC.getAdvanceWidth(...) method is being used to calculate character width which doesn't take zooming in consideration as it gets width using native OS method

public int getAdvanceWidth(char ch) {
	if (handle == 0) SWT.error(SWT.ERROR_GRAPHIC_DISPOSED);
	checkGC(FONT);
	int[] width = new int[1];
	OS.GetCharWidth(handle, ch, ch, width);
	return width[0];
}

Resolution
Used GC.textExtent() method which automatically handles zooming using DPIUtil.autoScaleDown(..) API

public Point textExtent (String string) {
	return DPIUtil.autoScaleDown(drawable, textExtentInPixels(string, SWT.DRAW_DELIMITER | SWT.DRAW_TAB));
}

Fixes #641

@mshahzadiftikhar
Copy link
Contributor Author

@jonahgraham kindly review this PR once you get some time. Thanks.

Copy link
Member

@jonahgraham jonahgraham left a comment

Choose a reason for hiding this comment

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

Thanks @mshahzadiftikhar this change looks good. It was really helpful that you included screen recordings to make it very easy to understand the problem and resolution.

Waiting for the CI to complete to make sure no version bumps or similar are needed and then we can proceed with the merge.

@jonahgraham
Copy link
Member

The build failed because it needs bumps on the modified bundles:

The following bundles are missing a service segment version bump:
  - org.eclipse.cdt.debug.ui.memory.floatingpoint
  - org.eclipse.cdt.debug.ui.memory.traditional
Please bump service segment by [10](https://github.com/eclipse-cdt/cdt/actions/runs/7286790709/job/19991019490?pr=645#step:9:11)0 if on main branch
The log of this build is above
See: https://wiki.eclipse.org/Version_Numbering#When_to_change_the_service_segment

I'll submit that change this time in a moment.

Cursor not behaving correctly in memory view editor while
chaging variable value on Hi-DPI #641

Used GC.textExtent() method for getting width of a character as
it handles zooming factor

Fixes #641
@jonahgraham
Copy link
Member

The other thing I did was reformat the commit message to meet standard git commit messaging formatting (short summary on line 1, blank line 2)

@jonahgraham jonahgraham added this to the 11.5.0 milestone Dec 27, 2023
@mshahzadiftikhar
Copy link
Contributor Author

Thanks @jonahgraham for reviewing this PR. CI is now failing for code cleanliness related checks: https://github.com/eclipse-cdt/cdt/pull/645/checks?check_run_id=19991696258

@jonahgraham
Copy link
Member

Test testSetInvalidCreatingDescription (org.eclipse.cdt.core.settings.model.CProjectDescriptionBasicTests) failed, but it is unrelated and a somewhat flaky test.

@jonahgraham jonahgraham merged commit 93c136d into eclipse-cdt:main Dec 27, 2023
3 of 4 checks passed
@jonahgraham
Copy link
Member

Thanks @mshahzadiftikhar for the fix. I really appreciate the detailed description and resolution.

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.

Cursor not behaving correctly in memory view editor while chaging variable value on Hi-DPI
2 participants