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: #1097 & #1198 Adding a UseLayoutRounding=True to the DialogHost #1199

Closed
wants to merge 3 commits into from

Conversation

Tokter
Copy link
Contributor

@Tokter Tokter commented Feb 26, 2019

seems to fix the issue reported in #1097 & #1198

@Tokter Tokter changed the title Adding a UseLayoutRounding=True to the DialogHost Fix: #1097 & #1198 Adding a UseLayoutRounding=True to the DialogHost Feb 26, 2019
@Keboo
Copy link
Member

Keboo commented Feb 26, 2019

Should this also get applied to the MaterialDesignEmbeddedDialogHost below?

@jespersh
Copy link
Contributor

https://docs.microsoft.com/en-us/dotnet/api/system.windows.frameworkelement.uselayoutrounding?view=netframework-4.7.2#remarks

You should set UseLayoutRounding to true on the root element. The layout system adds child coordinates to the parent coordinates; therefore, if the parent coordinates are not on a pixel boundary, the child coordinates are also not on a pixel boundary. If UseLayoutRounding cannot be set at the root, set SnapsToDevicePixels on the child to obtain the effect that you want.

Is DialogHost the root element? I forgot if it is.

@Tokter
Copy link
Contributor Author

Tokter commented Feb 26, 2019

@Keboo Yes, I added it as well.
@jespersh Not sure, but it creates a popup with its own Visual Tree, so I think so...

@Keboo
Copy link
Member

Keboo commented Feb 26, 2019

@jespersh that is an excellent point. Usually the dialog host is a root level element inside of the window but it doesn't have to be (the demo app shows this case).
By setting this on the DialogHost element directly it will propagate down to both the dialog itself as well as all of the control that are being wrapped by the DialogHost. Given that, and the suggestion from the docs about wanting to set this on the root element to handle the case where a nested element may not be on the pixel boundary, I think I would suggest the following (assuming I am understanding everything correctly):

  1. Set UseLayoutRounding on MaterialDesignDialogHostPopup. That way it will get applied to the Popup control (and I suspect resolve your issue as well).
  2. Avoid setting it on the DialogHost since there are no guarantee that it will exist on the pixel boundaries. If people know that it is what they want, based on how they are using the DialogHost in their app, they are free to derive from the MDIX style and add it.
  3. We should be able to ignore the embedded dialog host (sorry for my previous comment) since it lives in the same visual tree.

@Tokter
Copy link
Contributor Author

Tokter commented Feb 26, 2019

@Keboo Your suggestion in point 1 works, UseLayoutRounding appear on the root element:
image

But the embedded dialog is then still glitchy:
image

image

image

@Tokter
Copy link
Contributor Author

Tokter commented Feb 26, 2019

I guess you could say that if you use the embedded dialog, then its up to the user to add a UseLayoutRounding on the MainWindow.

@Keboo Keboo added the wiki This item has wiki changes label Feb 26, 2019
@Keboo
Copy link
Member

Keboo commented Feb 26, 2019

@Tokter yea I think simply documenting that behavior for the embedded dialog host is the way to go. I have been working on trying to update the wiki with lots of this knowledge.

@Keboo Keboo added this to the 2.6.0 milestone Apr 13, 2019
@Keboo
Copy link
Member

Keboo commented Jun 18, 2019

Wiki documentation added here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wiki This item has wiki changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants