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

The default value is not displayed in the correct position after setting SelectedObject property for PropertyGird control #4593

Closed
Jessie-Zhang01 opened this issue Feb 23, 2021 · 15 comments · Fixed by #4610 or #4611
Assignees
Labels
💥 regression-preview Regression from a preview release

Comments

@Jessie-Zhang01
Copy link

Jessie-Zhang01 commented Feb 23, 2021

  • .NET Core Version: .Net SDK 6.0.100-preview.2.21120.3, .Net SDK 5.0.104 from March 2021 Update Test Pass

  • Have you experienced this same bug with .NET Framework?:
    No

Problem description:
The default value is not displayed in the correct position after setting SelectedObject property for PropertyGird control. It seems to be displayed in the upper left corner of the propertygrid.
image
step1

Expected behavior:
After setting the SelectedObject property for the PropertyGird control, it should display correct default values.
image

More Info:
This is a regression issue, it cannot reproduce on .Net SDK 5.0.103 and .Net SDK 6.0.100-preview.2.21111.1, but can reproduce on .Net SDK 5.0.104 from March 2021 Update Test Pass and .Net SDK 6.0.100-preview.2.21112.1.

Minimal repro:

  1. Create a WinForms .Net Core application.
  2. Add a PropertyGird control to Form.
  3. Press F4 to go to the Property Browser Window, set its SelectedObject property to Form1.
  4. Build and run the application, observe the PropertyGrid.

Or

  1. Extract the attached project and open it.
    Test.zip

  2. Build and run this application, observe the PropertyGrid.

@Jessie-Zhang01 Jessie-Zhang01 added the 💥 regression-preview Regression from a preview release label Feb 23, 2021
@RussKie RussKie added the priority-0 Work that we can't release without label Feb 24, 2021
@RussKie
Copy link
Member

RussKie commented Feb 24, 2021

This is a regression surfaced by #4525. Before #4525 any modifications to a Graphics object would be correctly applied, as the code would follow this path:

public DeviceContextHdcScope(
IDeviceContext deviceContext,
bool applyGraphicsState = true,
bool saveHdcState = false) : this (

Since #4525 we rely on flags passed into DrawTextInternal completely side-stepping any modifications applied to a Graphics object. PropertyGrid performs clipping and transformations when rendering values:
protected virtual void DrawValueEntry(Graphics g, int row, ref Rectangle clipRect)
{
GridEntry gridEntry = GetGridEntryFromRow(row);
if (gridEntry is null)
{
return;
}
Debug.WriteLineIf(GridViewDebugPaint.TraceVerbose, "Drawing value for property " + gridEntry.PropertyLabel);
Rectangle r = GetRectangle(row, ROWVALUE);
Point newOrigin = new Point(r.X, r.Y);
Rectangle cr = Rectangle.Intersect(clipRect, r);
if (cr.IsEmpty)
{
return;
}
AdjustOrigin(g, newOrigin, ref r);
cr.Offset(-newOrigin.X, -newOrigin.Y);
try
{
try
{
gridEntry.PaintValue(
null,
g,
r,
cr,
GridEntry.PaintValueFlags.FetchValue
| GridEntry.PaintValueFlags.PaintInPlace
| GridEntry.PaintValueFlags.CheckShouldSerialize);
}
catch
{
}
}
finally
{
ResetOrigin(g);
}
}

It looks like an oversight (or a sad omission during the WindowsGraphics-related cleanups) that the value-rendering procedure doesn't explicitly request to preserve the graphics modifications:

User32.DT format = User32.DT.EDITCONTROL | User32.DT.EXPANDTABS | User32.DT.NOCLIP
| User32.DT.SINGLELINE | User32.DT.NOPREFIX;

TextRenderer.DrawTextInternal(
g,
text,
GetFont(boldFont: valueModified),
textRectangle,
textColor,
backColor,
(TextFormatFlags)format);

...whilst the label rendering code does:

TextRenderer.DrawText(g, strLabel, font, textRect, textColor, PropertyGrid.MeasureTextHelper.GetTextRendererFlags());

public static TextFormatFlags GetTextRendererFlags()
{
return TextFormatFlags.PreserveGraphicsClipping |
TextFormatFlags.PreserveGraphicsTranslateTransform;
}

This looks a fix:

diff --git a/src/System.Windows.Forms/src/System/Windows/Forms/PropertyGridInternal/GridEntry.cs b/src/System.Windows.Forms/src/System/Windows/Forms/PropertyGridInternal/GridEntry.cs
index 75b07f49c..41f095bd6 100644
--- a/src/System.Windows.Forms/src/System/Windows/Forms/PropertyGridInternal/GridEntry.cs
+++ b/src/System.Windows.Forms/src/System/Windows/Forms/PropertyGridInternal/GridEntry.cs
@@ -2389,7 +2389,7 @@ namespace System.Windows.Forms.PropertyGridInternal
                 textRectangle,
                 textColor,
                 backColor,
-                (TextFormatFlags)format);
+                (TextFormatFlags)format | PropertyGrid.MeasureTextHelper.GetTextRendererFlags());
 
             ValueToolTipLocation = doToolTip ? new Point(rect.X + 2, rect.Y - 1) : InvalidPoint;
         }
Before After
image image

May also worth adding this as a guard:

diff --git a/src/System.Windows.Forms/src/System/Windows/Forms/TextRenderer.cs b/src/System.Windows.Forms/src/System/Windows/Forms/TextRenderer.cs
index d05a1e8f5..05272586f 100644
--- a/src/System.Windows.Forms/src/System/Windows/Forms/TextRenderer.cs
+++ b/src/System.Windows.Forms/src/System/Windows/Forms/TextRenderer.cs
@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using System.Diagnostics;
 using System.Drawing;
 using System.Drawing.Text;
 using static Interop;
@@ -603,7 +604,7 @@ namespace System.Windows.Forms
         /// </summary>
         private static ApplyGraphicsProperties GetApplyStateFlags(IDeviceContext deviceContext, TextFormatFlags textFormatFlags)
         {
-            if (deviceContext is not Graphics)
+            if (deviceContext is not Graphics graphics)
             {
                 return ApplyGraphicsProperties.None;
             }
@@ -619,6 +620,16 @@ namespace System.Windows.Forms
                 apply |= ApplyGraphicsProperties.TranslateTransform;
             }
 
+            Debug.Assert(apply.HasFlag(ApplyGraphicsProperties.Clipping)
+                || graphics.Clip is null
+                || graphics.Clip.GetHrgn(graphics) == IntPtr.Zero,
+                "Must preserve Graphics clipping region!");
+
+            Debug.Assert(apply.HasFlag(ApplyGraphicsProperties.TranslateTransform)
+                || graphics.Transform is null
+                || graphics.Transform.IsIdentity,
+                "Must preserve Graphics transformation!");
+
             return apply;
         }
     }

@JeremyKuhne
Copy link
Member

Your fix looks right, and I like the assert.

@merriemcgaw
Copy link
Member

Agreed - we should get this in 5.0.5 for sure.

@kirsan31
Copy link
Contributor

kirsan31 commented Feb 25, 2021

Am I understand right, that this regression will be introduced in 5.0.4 anyway?

@RussKie
Copy link
Member

RussKie commented Feb 25, 2021 via email

@kirsan31
Copy link
Contributor

kirsan31 commented Feb 25, 2021

Thanks for the info (forewarned is forearmed). We need to stick with 5.0.3 then...
This is just that rare case when updating .Net with windows update is not very good ))))

@RussKie
Copy link
Member

RussKie commented Feb 25, 2021 via email

@RussKie
Copy link
Member

RussKie commented Feb 25, 2021

I've updated the known issues docs, these will be published when 5.0.4 and 6.0p2 are released

@ghost ghost added the 🚧 work in progress Work that is current in progress label Feb 25, 2021
@RussKie RussKie linked a pull request Mar 2, 2021 that will close this issue
RussKie added a commit to RussKie/winforms that referenced this issue Mar 2, 2021
The fix introduced in dotnet#4525 has redirected the `PropertyGrid` value
rendering through a different code path which only relies on flags passed
into `DrawTextInternal` completely side-stepping any modifications applied
to the `Graphics` object. However the `PropertyGrid` value rendering
routines do modify the `Graphics` object (applying clipping and
transformations) but do not pass the necessary flags to `DrawTextInternal`.

Fix by applying the necessary flags to preserve the `Graphics` modifications,
and add debug-time asserts to check that necessary flags are applies,
if the underlying `Graphics` object is modified.

Fixes dotnet#4593
RussKie added a commit that referenced this issue Mar 2, 2021
The fix introduced in #4525 has redirected the `PropertyGrid` value
rendering through a different code path which only relies on flags passed
into `DrawTextInternal` completely side-stepping any modifications applied
to the `Graphics` object. However the `PropertyGrid` value rendering
routines do modify the `Graphics` object (applying clipping and
transformations) but do not pass the necessary flags to `DrawTextInternal`.

Fix by applying the necessary flags to preserve the `Graphics` modifications,
and add debug-time asserts to check that necessary flags are applies,
if the underlying `Graphics` object is modified.

Fixes #4593
RussKie added a commit to RussKie/winforms that referenced this issue Mar 2, 2021
The fix introduced in dotnet#4525 has redirected the `PropertyGrid` value
rendering through a different code path which only relies on flags passed
into `DrawTextInternal` completely side-stepping any modifications applied
to the `Graphics` object. However the `PropertyGrid` value rendering
routines do modify the `Graphics` object (applying clipping and
transformations) but do not pass the necessary flags to `DrawTextInternal`.

Fix by applying the necessary flags to preserve the `Graphics` modifications,
and add debug-time asserts to check that necessary flags are applies,
if the underlying `Graphics` object is modified.

Fixes dotnet#4593

(cherry picked from commit 34cdcf6)
@kirsan31
Copy link
Contributor

kirsan31 commented Mar 10, 2021

@RussKie

I've updated the known issues docs, these will be published when 5.0.4 and 6.0p2 are released

I think the doc must point out that this is only 5.0.4 issue. Сurrent text indicates an error in all versions up to 5.0.5...

@RussKie
Copy link
Member

RussKie commented Mar 10, 2021

The changelog is updated to reflect the state of the current release.

@kirsan31
Copy link
Contributor

The changelog is updated to reflect the state of the current release.

Forgot to post link of the doc - sorry :( I meant this: https://github.com/dotnet/core/blob/main/release-notes/5.0/5.0-known-issues.md
image

RussKie added a commit that referenced this issue Mar 10, 2021
The fix introduced in #4525 has redirected the `PropertyGrid` value
rendering through a different code path which only relies on flags passed
into `DrawTextInternal` completely side-stepping any modifications applied
to the `Graphics` object. However the `PropertyGrid` value rendering
routines do modify the `Graphics` object (applying clipping and
transformations) but do not pass the necessary flags to `DrawTextInternal`.

Fix by applying the necessary flags to preserve the `Graphics` modifications,
and add debug-time asserts to check that necessary flags are applies,
if the underlying `Graphics` object is modified.

Fixes #4593

(cherry picked from commit 34cdcf6)
@ghost ghost removed the 🚧 work in progress Work that is current in progress label Mar 10, 2021
@kirsan31
Copy link
Contributor

kirsan31 commented Mar 10, 2021

@RussKie Any advice how to specify exact runtime version with framework-dependent applications?
RuntimeFrameworkVersion doesn't work :(((

For framework-dependent applications, the RuntimeFrameworkVersion specifies the minimum required runtime framework version.

Initially, I didn't read the documentation carefully and was sure that the RuntimeFrameworkVersion is what we need...

OMG I didn't realize the magnitude of the whole problem - 5.0.3 will be overwritten with 5.0.4 by WU anyway. We have big big troubles now... 😭

Am I understand right, that on machines where 5.0.4 will be installed the only options are:

  1. self-contained deployment.
  2. manually install 5.0.3 and delete 5.0.4 from c:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App\.

?

@RussKie
Copy link
Member

RussKie commented Mar 17, 2021

I'm afraid I don't know enough about the .NET deployments via the WU. Manually it is possible to install the runtimes side-by-side.
Please direct any questions about the WU to the dotnet/sdk repo.

You can define how you wish the runtime to be resolved, please check https://github.com/dotnet/runtime/blob/main/docs/design/features/framework-version-resolution.md
You may also find this useful: https://docs.microsoft.com/dotnet/core/deploying/runtime-patch-selection

@RussKie RussKie removed the priority-0 Work that we can't release without label Mar 17, 2021
@Zheng-Li01
Copy link
Member

Verified the issue with latest 6.0.100-preview.3.21169.6, the issue has been fixed that the PropertyGird control display correctly as below screenshot.
image

@Jessie-Zhang01
Copy link
Author

Verified the issue with .Net SDK 5.0.202 from April 2021 Update Test Pass, this issue is fixed.
image

@ghost ghost locked as resolved and limited conversation to collaborators Jan 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
💥 regression-preview Regression from a preview release
Projects
None yet
6 participants