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

Fix the what the IgnorePixelScaling property works #1804

Merged
merged 17 commits into from
Sep 12, 2021

Conversation

mattleibow
Copy link
Contributor

Description of Change

Previously the IgnorePixelScaling allowed the drawing code to work in density-independent UI units, however it did so in a poor manner. Instead of scaling the canvas in such a way that it preserved the high density, it stretched and reduced the quality of the image.

This PR makes sure to always draw in high resolution, and then pre-scale the canvas before invoking the paint events.

Bugs Fixed

API Changes

None.

Behavioral Changes

A demo of the change.

Before After
image image

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Rebased on top of main at time of PR
  • Changes adhere to coding standard
  • Updated documentation

@mattleibow mattleibow added this to In progress in v2.88.0 via automation Sep 10, 2021
@mattleibow mattleibow changed the title Fix the wat the IgnorePixelScaling property works Fix the what the IgnorePixelScaling property works Sep 10, 2021
@myroot
Copy link
Contributor

myroot commented Sep 11, 2021

Hi, this PR meet the our requirements except one thing

We hope to able to update ScalingFactor

public static double ScalingFactor => scalingFactor.Value;

Because, in several reasons, we need to support a virtual scaling factor.
Could you make a setter on ScalingInfo.ScalingFactor?

@mattleibow
Copy link
Contributor Author

mattleibow commented Sep 11, 2021

Because, in several reasons, we need to support a virtual scaling factor.

@myroot I can do that. Do we need to have different scaling factors on different canvases? Or can it all be in one place?

For now I will add a setter to the ScalingInfo type. But we can easily move it to the canvas before we merge this.

Do you have an example of when you would want the scaling to be different to the screen DPI?

Comment on lines +38 to +41
public static void SetScalingFactor(double? scalingFactor)
{
scalingFactorOverride = scalingFactor;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding this as a separate method so it can be a null that can allow for reverting back to the system value.

Is this useful in this global context? Should it be on the individual canvas views?

Copy link
Contributor

Choose a reason for hiding this comment

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

Good, We apply custom scale as globally, so It works for tizen.

@myroot
Copy link
Contributor

myroot commented Sep 11, 2021

Do you have an example of when you would want the scaling to be different to the screen DPI?

https://github.com/Samsung/Tizen.UIExtensions/blob/d13d130b552a177939d81a12cacf7069c343fa6c/src/Tizen.UIExtensions.ElmSharp/DeviceInfo.cs#L233

Here is a code of setting custom scaling factor on tizen. if this PR was merged, I will add code to call SetScalingFactor in the end of this method

@mattleibow mattleibow merged commit 4a687a8 into main Sep 12, 2021
v2.88.0 automation moved this from In progress to Done Sep 12, 2021
@mattleibow mattleibow deleted the dev/improve-view-scaling branch September 12, 2021 07:32
@mattleibow mattleibow added this to the v2.88.0 milestone May 22, 2022
@tipa
Copy link

tipa commented Dec 13, 2022

@mattleibow I just stumbled over this and how the IgnorePixelScaling works.
From what I understand the new behavior is different from the document behavior, which was the old one.
The docs still say When true, the canvas is resized to device independent pixels, and then stretched to fill the view.
This isn't the case any more with v2.88.0, is it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[FEATURE] Rework the IgnorePixelScaling property
3 participants