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

Consider Changing CombineRgn Return Type to Enum #1024

Closed
lonitra opened this issue Aug 2, 2022 · 4 comments
Closed

Consider Changing CombineRgn Return Type to Enum #1024

lonitra opened this issue Aug 2, 2022 · 4 comments
Labels
enhancement New feature or request missing enum An enum is missing for constant parameters

Comments

@lonitra
Copy link
Member

lonitra commented Aug 2, 2022

Is your feature request related to a problem? Please describe.
The current return type of CombineRgn is an int, which affects code readability since getting a 0 as a return may not immediately signify to the reader that there was an error, and no region was created. To work around this, users could create their own enum type, but even then they would still need to cast their enum type everywhere they want to check the return of CombineRgn.

Describe the solution you'd like
Have CombineRgn return an enum type for readability and cleanliness. Perhaps something like:

internal enum RegionType : int
    {
        ERROR = 0,
        NULLREGION = 1,
        SIMPLEREGION = 2,
        COMPLEXREGION = 3,
    }

This enum type could also be used for the return type of other methods such as GetClipBox, GetRgnBox, IntersectClipRect etc.

@AArnott
Copy link
Member

AArnott commented Aug 3, 2022

Your enum looks good. The 3 non-zero values are already defined as constants in the metadata and could be relocated to the enum. The value for ERROR should be defined natively in the enum rather than relocating it (if it's in the metadata) since it obviously isn't named specifically for the region APIs and probably is used elsewhere.

@AArnott AArnott added missing enum An enum is missing for constant parameters and removed enhancement New feature or request labels Aug 3, 2022
@AArnott AArnott transferred this issue from microsoft/CsWin32 Aug 3, 2022
@mikebattista
Copy link
Contributor

Windows.Win32.Graphics.Gdi.GDI_REGION_TYPE not found in 1st winmd
Windows.Win32.Graphics.Gdi.Apis.ERROR...System.UInt32 => System.Int32
Windows.Win32.Graphics.Gdi.Apis.NULLREGION not found in 2nd winmd
Windows.Win32.Graphics.Gdi.Apis.SIMPLEREGION not found in 2nd winmd
Windows.Win32.Graphics.Gdi.Apis.COMPLEXREGION not found in 2nd winmd
Windows.Win32.Graphics.Gdi.Apis.RGN_ERROR not found in 2nd winmd
Windows.Win32.Graphics.Gdi.Apis.CombineRgn : return...Int32 => GDI_REGION_TYPE
Windows.Win32.Graphics.Gdi.Apis.ExcludeClipRect : return...Int32 => GDI_REGION_TYPE
Windows.Win32.Graphics.Gdi.Apis.GetClipBox : return...Int32 => GDI_REGION_TYPE
Windows.Win32.Graphics.Gdi.Apis.GetRgnBox : return...Int32 => GDI_REGION_TYPE
Windows.Win32.Graphics.Gdi.Apis.IntersectClipRect : return...Int32 => GDI_REGION_TYPE
Windows.Win32.Graphics.Gdi.Apis.OffsetClipRgn : return...Int32 => GDI_REGION_TYPE
Windows.Win32.Graphics.Gdi.Apis.OffsetRgn : return...Int32 => GDI_REGION_TYPE
Windows.Win32.Graphics.Gdi.Apis.SelectClipRgn : return...Int32 => GDI_REGION_TYPE
Windows.Win32.Graphics.Gdi.Apis.ExtSelectClipRgn : return...Int32 => GDI_REGION_TYPE
Windows.Win32.Graphics.Gdi.Apis.SetMetaRgn : return...Int32 => GDI_REGION_TYPE

@mikebattista
Copy link
Contributor

I left ERROR separate for now and added RGN_ERROR to the enum which was an existing alias for ERROR.

@mikebattista
Copy link
Contributor

I'm planning to ship a metadata update later this week so if there are other issues affecting WinForms, let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request missing enum An enum is missing for constant parameters
Projects
None yet
Development

No branches or pull requests

3 participants