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

Add implicit conversion from RECT -> System.Drawing.Rectangle (System.Drawing.Primitives v8.0.0 from base shared framework). #387

Open
AraHaan opened this issue Mar 2, 2024 · 4 comments · May be fixed by #388
Labels
proposal An issue that represents a proposed feature or change to the repo. untriaged An issue that has not been triaged by the repo maintainers.

Comments

@AraHaan
Copy link

AraHaan commented Mar 2, 2024

Description (optional)

I think an helpful implicit conversion to Rectangle would help with everything with APIs that uses RECT or returns RECT.

Rationale

Why to System.Drawing.Rectangle? Because the type exists in System.Drawing.Primitives which is part of the base shared framework which means it is not locked down to Microsoft.WindowDesktop.App as it exists in the Microsoft.NETCore.App shared framework which means it is available everywhere for every operating system. With it existing as a reference to every project that references the framework without it disabled that means it is safe to use in TerraFX.Interop.Windows as it is a primitive type. Also having operators between the 2 will help with RECT <-> Rectangle conversions without things going wrong when the developer messes up conversions as the operators will take all the guess work out of it all.

Proposed API

namespace TerraFX.Interop.Windows;

public partial struct RECT
{
+    public static implicit operator Rectangle(RECT rectangle);
+    public static explicit operator RECT(Rectangle rectangle);
}

Drawbacks

I think an implicit conversion like this would have minimal drawbacks with an explicit cast from Rectangle -> RECT as well so people can also pass those to APIs without doing it the long way as well (it becomes problematic to remember what should go into the Left, Right, Top, Bottom so operators that handles it for us could also help prevent bugs in people's code as well as convenience as well.

Alternatives

I have not though of any alternatives yet.

Other thoughts

Implementation would look something like this of the new operators:

namespace TerraFX.Interop.Windows;

public partial struct RECT
{
    public static implicit operator Rectangle(RECT rectangle)
        => Rectangle.FromLTRB(rectangle.left, rectangle.top, rectangle.right, rectangle.bottom);

    public static explicit operator RECT(Rectangle rectangle)
        => new // or new RECT()
        {
            // Obtained from inspecting 'Rectangle.FromLTRB(int, int, int, int)'.
            left = rectangle.X,
            top = rectangle.Y,
            right = rectangle.Width + rectangle.X,
            bottom = rectangle.Height + rectangle.Y,
        };
}

Discussions (optional)

I have not discussed about this much, this idea just came into mind when I was looking at some of my old code that processes TitleBarInfo related things.

@AraHaan AraHaan added proposal An issue that represents a proposed feature or change to the repo. untriaged An issue that has not been triaged by the repo maintainers. labels Mar 2, 2024
AraHaan added a commit to AraHaan/terrafx.interop.windows that referenced this issue Mar 2, 2024
Fixes terrafx#387.

Signed-off-by: AraHaan <seandhunt_7@yahoo.com>
@rickbrew
Copy link
Contributor

rickbrew commented Mar 2, 2024

My objection to having this in TerraFX is that RECT and Rectangle do not have the same representation, and cannot always be converted correctly.

RECT is { int left, top, right, bottom; } while Rectangle is { int x, y, width, height; }

Consider a RECT equal to { int.MinValue, int.MinValue, int.MaxValue, int.MaxValue }. This can't be represented as a Rectangle because the width/height is int.MaxValue - int.MinValue and that overflows. So should this throw an exception? Should it just silently overflow? Should it be clamped, and if so to what range? IMO this should be left to applications to implement on their own, because the answer to these questions is application-specific.

Also, FWIW, WICRect is bit-compatible with Rectangle.

@AraHaan
Copy link
Author

AraHaan commented Mar 3, 2024

My objection to having this in TerraFX is that RECT and Rectangle do not have the same representation, and cannot always be converted correctly.

RECT is { int left, top, right, bottom; } while Rectangle is { int x, y, width, height; }

Consider a RECT equal to { int.MinValue, int.MinValue, int.MaxValue, int.MaxValue }. This can't be represented as a Rectangle because the width/height is int.MaxValue - int.MinValue and that overflows. So should this throw an exception? Should it just silently overflow? Should it be clamped, and if so to what range? IMO this should be left to applications to implement on their own, because the answer to these questions is application-specific.

Also, FWIW, WICRect is bit-compatible with Rectangle.

I am sure in the case of TITLEBARINFOEX the conversion from RECT -> Rectangle is fine because in my code it never seems to overflow (that I know of so far).

ThemedForm.cs
public class ThemedForm : Form
{
    /* snip system menu -> ContextMenuStrip conversion functions. */

    internal static unsafe TitleBarInfo GetTitleBarInfo(HWND hwnd)
    {
        var tbInfoEx = new TITLEBARINFOEX
        {
            cbSize = (uint)Unsafe.SizeOf<TITLEBARINFOEX>(),
        };
        
        // I am considering using "fixed" here with GC.KeepAlive() on the pointer and pray that the resulting TITLEBARINFOEX
        that was passed into this call to SendMessageW would be properly filled in with enough information without actually handing the WM.WM_GETTITLEBARINFOEX message within WndProc myself.
        _ = Windows.SendMessageW(hwnd, WM.WM_GETTITLEBARINFOEX, 0u, new IntPtr(&tbInfoEx));
        return new TitleBarInfo(tbInfoEx);
    }

    protected override unsafe void WndProc(ref Message m)
    {
        if ((m.Msg is WM.WM_SYSCOMMAND
            && m.WParam.ToInt32() is SC.SC_MOUSEMENU or SC.SC_KEYMENU
            /* Currently is not what I expect to happen on Windows 11 instead of SC_MOUSEMENU.*/
            or 61587)
            /* Undocumented but works for "Taskbar right clicks". */
            || m.Msg is 0x313
            /* To handle right click in NC area. */
            || m.Msg is WM.WM_CONTEXTMENU)
        {
            // this here is when I basically take the cached "System" Menu that was converted it to a dark themed "ContextMenuStrip" that I then show it here. This strip has a custom renderer assigned to it where I use m.LParam to get the X and Y coordinates to display it.
            return;
        }
        else if (m.Msg is WM.WM_NCPAINT)
        {
            if (this.active)
            {
                var tbInfo = GetTitleBarInfo((HWND)m.HWnd);
                using var graphics = Graphics.FromHwnd(m.HWnd);
                using var pen = new Pen(ApplicationResources.Theme!.BorderColor);

                // draw on the "Title Bar".
                // for some reason I cant see it though on Windows 11.
                graphics.DrawRectangle(pen, Rectangle.Inflate(tbInfo.Items[0].Bounds, -1, -1));

                // TODO: Get bounds of the NC area TEXT and paint over it in white.
                // return;
            }
        }
        else if (m.Msg is WM.WM_NCACTIVATE)
        {
            this.active = m.WParam.ToInt32().Equals(Convert.ToInt32(true));
            var tbInfo = GetTitleBarInfo((HWND)m.HWnd);
            fixed (RECT* pRect = &tbInfo.Items[0].Rect)
            {
                _ = Windows.InvalidateRect((HWND)m.HWnd, pRect, true);
            }

            if (this.active)
            {
                return;
            }
        }

        base.WndProc(ref m);
    }

    /* structure named MENUITEMINFO that holds an MENUITEMINFOW with helper functions to convert them to ToolStripItem */

    internal struct TitleBarInfo
    {
        public unsafe TitleBarInfo(TITLEBARINFOEX tbInfo)
        {
            this.Items = new ItemInfo[6];
            for (var i = 0; i < this.Items.Length; i++)
            {
                this.Items[i].State = (ItemState)tbInfo.rgstate[i];
                this.Items[i].Rect = tbInfo.rgrect[i];
                this.Items[i].Bounds = Rectangle.FromLTRB(
                    this.Items[i].Rect.left,
                    this.Items[i].Rect.top,
                    this.Items[i].Rect.right,
                    this.Items[i].Rect.bottom);
                switch (i)
                {
                    case 1: // Reserved.
                        break;
                    case 2:
                        this.Items[i].IsMinimizeButton = true;
                        break;
                    case 3:
                        this.Items[i].IsMaximizeButton = true;
                        break;
                    case 4:
                        this.Items[i].IsHelpButton = true;
                        break;
                    case 5:
                        this.Items[i].IsCloseButton = true;
                        break;
                    default:
                        break;
                }
            }

            // replace item 0's bounds with rcTitleBar.
            this.Items[0].Rect = tbInfo.rcTitleBar;
            this.Items[0].Bounds = Rectangle.FromLTRB(
                this.Items[0].Rect.left,
                this.Items[0].Rect.top,
                this.Items[0].Rect.right,
                this.Items[0].Rect.bottom);
        }

        [Flags]
        internal enum ItemState : uint
        {
            STATE_SYSTEM_FOCUSABLE = 0x00100000,
            STATE_SYSTEM_INVISIBLE = 0x00008000,
            STATE_SYSTEM_OFFSCREEN = 0x00010000,
            STATE_SYSTEM_UNAVAILABLE = 0x00000001,
            STATE_SYSTEM_PRESSED = 0x00000008,
        }

        public ItemInfo[] Items { get; set; }

        internal struct ItemInfo
        {
            public RECT Rect;

            public ItemState State { get; set; }

            public Rectangle Bounds { get; set; }

            public readonly Size Size
                => this.Bounds.Size;

            public readonly Point Location
                => this.Bounds.Location;

            public bool IsCloseButton { get; set; }

            public bool IsHelpButton { get; set; }

            public bool IsMinimizeButton { get; set; }

            public bool IsMaximizeButton { get; set; }
        }
    }
}

That code, I plan to eventually use to have a manual drawn title bar (non-client area) that can be themed to any colors even non-system wide ones similar to how office 2010 for example did with a fake non-client area using the information like the Size of the specific parts of it and then calling windows apis to get the images rendered inside of say the buttons themselves. With that the real ones would need to be "disabled" (aka turned off to never render) for it to work then.

@tannergooding
Copy link
Member

tannergooding commented Mar 3, 2024

I'd tend to agree with Rick here, but additionally, I don't really want to unnecessarily bring in new dependencies, particularly not ones that could require me to expose my TFM as net*-windows. -- I know Rectangle is currently in System.Drawing.Primitives and so doesn't require this, but most of System.Drawing is not in that camp and this issue would open up a precedent that then has to be discussed for other similar cases where an "in-box" type exists.

This seems like a good place for a user to provide some static Rectangle ToRectangle(this RECT rect) (and the inverse) extension API where they can ensure that the conversion works for their needs (including overflow behavior) -- noting that because the conversion is potentially lossy, the conversion APIs would have to be explicit and so you don't really get anything from it being an operator on RECT at that point.

@rickbrew
Copy link
Contributor

rickbrew commented Mar 3, 2024

Having an implicit conversion from RECT to Rectangle would be a bug hazard -- way too easy to accidentally mix them together and end up with silent overflows that cause bugs or worse.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal An issue that represents a proposed feature or change to the repo. untriaged An issue that has not been triaged by the repo maintainers.
Projects
None yet
3 participants