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

Make methods in the UITheme hierarchy take the display scale factor into account #14464

Merged

Conversation

Rinzwind
Copy link
Contributor

@Rinzwind Rinzwind commented Aug 9, 2023

This pull request makes methods in the UITheme hierarchy take the display scale factor into account.

@Rinzwind
Copy link
Contributor Author

Rinzwind commented Aug 9, 2023

Pull request for one method that is ‘NewTools’: pharo-spec/NewTools#576

@jecisc
Copy link
Member

jecisc commented Aug 10, 2023

Wouldn't it be simpler to manage the display scale factor in the Morph instead of the UITheme?

@Rinzwind
Copy link
Contributor Author

I’m not sure I understand what you have in mind, but maybe this helps: one problem to avoid is that values get double scaled, as would happen when passing the value of an accessor of one morph to a mutator of another morph if that mutator also scales it, a way to avoid that double scaling problem is to only scale constants.

@Rinzwind
Copy link
Contributor Author

Rinzwind commented Aug 12, 2023

I do think it would make sense to handle the display scale factor somewhere else though. A problem that exists with it now is that things don’t adapt when it is changed. Take this snippet which opens a PolygonMorph:

(PolygonMorph
	vertices: (((10@10) scaledByDisplayScaleFactor extent: (30@30) scaledByDisplayScaleFactor)
		in: [ :rectangle |
			{ rectangle bottomLeft. rectangle topCenter. rectangle bottomRight } ])
	color: Color transparent
	borderWidth: 1 scaledByDisplayScaleFactor
	borderColor: Color black) openInWorld.

In a new Pharo image, after disabling #showMenubar and #showDesktopLogo, the morph looks like this:

If I change the world scale factor and the display scale factor to ‘Retina display’ scaling:

World scaleFactor: 1/2.
WorldMorph displayScaleFactor: 2.

It does not automatically look like this:

For that, I have to do the first snippet again to open a new PolygonMorph. To make it automatic, I think it might make sense to handle the display scale factor in the canvas. I’m not sure if that has been considered before and if so, what problems that was expected to have. Perhaps someone who has worked on it before can answer that, the concept of display scale factor was introduced in commit 0b3ddf1 as part of pull request #1461, and there are a few other pull requests and issues that mention ‘displayScaleFactor’ or ‘display scale factor’, though many of them are the ones I opened over the past few months.

For an ImageMorph:

(ImageMorph withForm: (ThemeIcons current iconNamed: #pharo))
	position: (10@10) scaledByDisplayScaleFactor;
	openInWorld

To automatically go from this:

To this:

I expect an intermediary between the ImageMorph and the Form would need to be introduced, so that a specific Form for each display scale factor can be used, like an object of a class Image which holds a Form for each display scale factor. This would be a bit similar to the separation between NSImage and NSImageRep in macOS’s AppKit I think.

For a TextMorph:

TextMorph new
	contents: (Text string: 'Lorem ipsum'
		attribute: (TextFontReference toFont:
			(LogicalFont familyName: 'Source Sans Pro' pointSize: 15 scaledByDisplayScaleFactor)));
	position: (10@10) scaledByDisplayScaleFactor;
	openInWorld.

To automatically go from this:

To this:

I’m not sure whether there’s anything in particular to take into account.

@Ducasse Ducasse merged commit ebbd0c1 into pharo-project:Pharo12 Aug 14, 2023
1 check passed
@Rinzwind Rinzwind deleted the uitheme-display-scale-factor branch August 29, 2023 15:15
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