8000 Fix get TransformToDevice in Stylus Input thread will throw the InvalidOperationException by lindexi · Pull Request #6840 · dotnet/wpf · GitHub
[go: up one dir, main page]
Skip to content

Fix get TransformToDevice in Stylus Input thread will throw the InvalidOperationException #6840

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Fix get TransformToDevice in Stylus Input thread will throw the InvalidOperationException #6840

wants to merge 3 commits into from

Conversation

lindexi
Copy link
Member
@lindexi lindexi commented Jul 22, 2022 •
edited
Loading

Fixes #6829

Description

See #6829

The Stylus Input thread will enter the GetAndCacheTransformToDeviceMatrix method to get the Matrix. But after we change the DPI, the Stylus Input thread can not find the Matrix in _transformToDeviceMatrices. It means that the Stylus Input thread should get the Matrix from HwndTarget.TransformToDevice property which only allow get from UI Thread.

I fix it by dispatch to UI Thread. To improve performance, we will calculate it CurrentDpiScale property.

Customer Impact

Fix tocuh in Window with StylusPlugIn may throw InvalidOperationException

Regression

Yes, see #2891

Testing

Just CI

And I pick the code to dotnet-campus/dotnetCampus.CustomWpf#21 to build the private version to test it by my application.

Risk

Low.

Microsoft Reviewers: Open in CodeFlow

@lindexi lindexi requested a review from a team as a code owner July 22, 2022 01:33
@ghost ghost assigned lindexi Jul 22, 2022
@ghost ghost added the PR metadata: Label to tag PRs, to facilitate with triage label Jul 22, 2022
@ghost ghost requested review from dipeshmsft, singhashish-wpf and SamBent July 22, 2022 01:33
@ghost ghost added the Community Contribution A label for all community Contributions label Jul 22, 2022
@lindexi lindexi mentioned this pull request Jul 22, 2022
Open
miloush
miloush reviewed Jul 27, 2022
View reviewed changes
src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/Input/Stylus/Common/StylusLogic.cs
_transformToDeviceMatrices[hwndSource.CompositionTarget.CurrentDpiScale] = hwndSource.CompositionTarget.TransformToDevice;
Debug.Assert(_transformToDeviceMatrices[hwndSource.CompositionTarget.CurrentDpiScale].HasInverse);
DpiScale2 currentDpiScale = compositionTarget.CurrentDpiScale;
toDevice.Scale(currentDpiScale.DpiScaleX, currentDpiScale.DpiScaleY);
Copy link
Contributor
@miloush miloush Jul 27, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

is the transform not cached?

Copy link
Member Author
@lindexi lindexi Jul 27, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@miloush Do you think cache is fast than calculation? Thank you. I means that we should handle the cross thread issues when we cache it.

@miloush
Copy link
Contributor
miloush commented Jul 27, 2022

I am wondering why is the class a DispatcherObject and whether the special case for HwndTarget is worth the performance gain. If HwndTarget is updated in the future to support other kinds of transforms, someone will have to magically remember 8000 that this code needs to be updated.

@lindexi
Copy link
Member Author
lindexi commented Jul 27, 2022 •
edited
Loading

If HwndTarget is updated in the future to support other kinds of transforms, someone will have to magically remember that this code needs to be updated.

@miloush In fact, I think it is more correct to use CurrentDpiScale. Thank you.

Why we use TransformToDevice here? Cc @rladuca

@lindexi lindexi requested a review from miloush July 29, 2022 01:09
@rchauhan18
Copy link
Contributor

Hey @lindexi ,
I'm having trouble reproducing this issue using the steps you provided. I might be overlooking something, so could you assist me by sharing a sample repository to clarify?

Here is the code I am using: 
public partial class MainWindow : Window
{
    public MainWindow()
    {
        InitializeComponent();
        
        this.StylusPlugIns.Add(new CustomStylusPlugin());
    }
   
}

public class CustomStylusPlugin : StylusPlugIn
{
    protected override void OnStylusDown(RawStylusInput rawStylusInput)
    {
        base.OnStylusDown(rawStylusInput);

        // Get the points
        StylusPointCollection stylusPoints = rawStylusInput.GetStylusPoints();

        // Output points
        foreach (StylusPoint point in stylusPoints)
        {
            System.Diagnostics.Debug.WriteLine($"X: {point.X}, Y: {point.Y}");
        }
    }
}

@lindexi
Copy link
Member Author
lindexi commented Nov 24, 2023 •
edited
Loading

@rchauhan18 Do you change the DPI? Please make sure the _transformToDeviceMatrices do not contain the CurrentDpiScale.

And could you debug and add the break point in GetAndCacheTransformToDeviceMatrix method? Can the Stylus Input thread enter the GetAndCacheTransformToDeviceMatrix method? If the Stylus Input thread can enter the GetAndCacheTransformToDeviceMatrix method, it means that the Stylus Input thread will visit the hwndSource.CompositionTarget.TransformToDevice which will throw InvalidOperationException.

@rchauhan18
Copy link
Contributor

@lindexi Yes, I am able to add a break point in GetAndCacheTranformToDeviceMatrix method. However, the issue I'm encountering is that when I change the DPI value from the settings, the application's DPI value doesn't update dynamically. To see the change, I have to restart the application. Therefore, the count always remains at one.

@lindexi
Copy link
Member Author
lindexi commented Nov 24, 2023

@rchauhan18 How about multi-screen with different DPI configuration? Moving the app between the two screens.

@rchauhan18
Copy link
Contributor

/azp run

@azure-pipelines Azure Pipelines
Copy link
Azure Pipelines successfully started running 1 pipeline(s).

@LickBag
Copy link
LickBag commented Jul 3, 2024

@rchauhan18 Do you change the DPI? Please make sure the _transformToDeviceMatrices do not contain the CurrentDpiScale.

And could you debug and add the break point in GetAndCacheTransformToDeviceMatrix method? Can the Stylus Input thread enter the GetAndCacheTransformToDeviceMatrix method? If the Stylus Input thread can enter the GetAndCacheTransformToDeviceMatrix method, it means that the Stylus Input thread will visit the hwndSource.CompositionTarget.TransformToDevice which will throw InvalidOperationException.

Stylus Input Thread
I met this problem, the Stylus Input thread can enter the GetAndCacheTransformToDeviceMatrix method, and throw the InvalidOperationException.
Please Fix It.

@lindexi
Copy link
Member Author
lindexi commented Jul 3, 2024

@LickBag Could you test my custom WPF version?

  1. Install or download the assets resouce from dotnetCampus.WPF.Resource package in your .NET 6 project.
    <PackageVersion Include="dotnetCampus.WPF.Resource" Version="6.0.4-alpha07-test06" />

  1. Publish your project with self contained

  2. Copy and replace the dll from C:\Users\<Your UserName>\.nuget\packages\dotnetcampus.wpf.resource\6.0.4-alpha07-test06\Assets\ folder
< 8000 !-- -->
lindexi referenced this pull request in dotnet-campus/wpf Nov 13, 2024
@lindexi
Try fix the first point in StylusPlugin in high DPI
7282478 
dotnet#6415
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dipeshmsft dipeshmsft Awaiting requested review from dipeshmsft

@singhashish-wpf singhashish-wpf Awaiting requested review from singhashish-wpf

@SamBent SamBent Awaiting requested review from SamBent

@miloush miloush Awaiting requested review from miloush

At least 1 approving review is required to merge this pull request.

Assignees

@lindexi lindexi

Labels
Community Contribution A label for all community Contributions Included in test pass PR metadata: Label to tag PRs, to facilitate with triage Status:Completed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

WPF tocuh in Window with StylusPlugIn may throw InvalidOperationException
6 participants
@lindexi @miloush @rchauhan18 @LickBag @harshit7962 @dipeshmsft
0