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

[API Proposal]: DrawRectangle should accept RectangleF as one of the overloads #62401

Closed
medo64 opened this issue Dec 4, 2021 · 7 comments
Closed
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-System.Drawing in-pr There is an active PR which will close this issue when it is merged
Milestone

Comments

@medo64
Copy link
Contributor

medo64 commented Dec 4, 2021

Background and motivation

In Graphics class, FillRectangle contains the following 4 overloads:

  • public void FillRectangle(Brush brush, RectangleF rect)
  • public void FillRectangle(Brush brush, float x, float y, float width, float height)
  • public void FillRectangle(Brush brush, Rectangle rect)
  • public void FillRectangle(Brush brush, int x, int y, int width, int height)

However, DrawRectangle, contains only the following 3 overloads:

  • public void DrawRectangle(Pen pen, Rectangle rect)
  • public void DrawRectangle(Pen pen, float x, float y, float width, float height)
  • public void DrawRectangle(Pen pen, int x, int y, int width, int height)

In the interest of symmetry, I propose adding the "missing" RectangleF overload:

  • public void DrawRectangle(Pen pen, RectangleF rect)

During a development process one can quite often switch between DrawRectangle and FillRectangle API. This would allow to do so without changing the variables around, i.e. just by changing Fill into Draw and backward. Likewise, this would make drawing filled rectangles with outline in different color minutely easier.

API Proposal

namespace System.Drawing.Common
{
    public class Graphics
    {
        public void DrawRectangle(Pen pen, RectangleF rect);
    }
}

API Usage

API usage would be comparable to the existing API:

var rect = new RectangleF(...);
e.Graphics.FillRectangle(Brushes.Blue, rect);  // this works even now
e.Graphics.DrawRectangle(Pens.Red, rect);      // this would be a new overload

Alternative Designs

No response

Risks

Since method will just be a convenient way of calling already existing overload, risk is minimal if any.

@medo64 medo64 added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Dec 4, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Drawing untriaged New issue has not been triaged by the area owner labels Dec 4, 2021
@ghost
Copy link

ghost commented Dec 4, 2021

Tagging subscribers to this area: @dotnet/area-system-drawing
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

In Graphics class, FillRectangle contains the following 4 overloads:

  • public void FillRectangle(Brush brush, RectangleF rect)
  • public void FillRectangle(Brush brush, float x, float y, float width, float height)
  • public void FillRectangle(Brush brush, Rectangle rect)
  • public void FillRectangle(Brush brush, int x, int y, int width, int height)

However, DrawRectangle, contains only the following 3 overloads:

  • public void DrawRectangle(Pen pen, Rectangle rect)
  • public void DrawRectangle(Pen pen, float x, float y, float width, float height)
  • public void DrawRectangle(Pen pen, int x, int y, int width, int height)

In the interest of symmetry, I propose adding the "missing" RectangleF overload:

  • public void DrawRectangle(Pen pen, RectangleF rect)

During a development process one can quite often switch between DrawRectangle and FillRectangle API. This would allow to do so without changing the variables around, i.e. just by changing Fill into Draw and backward. Likewise, this would make drawing filled rectangles with outline in different color minutely easier.

API usage would be comparable to the existing API:

var rect = new RectangleF(...);
e.Graphics.FillRectangle(Brushes.Blue, rect);  // this works even now
e.Graphics.DrawRectangle(Pens.Red, rect);      // this would be a new overload

API Proposal

namespace System.Collections.Generic
{
    public class MyFancyCollection<T> : IEnumerable<T>
    {
        public void Fancy(T item);
    }
}

API Usage

// Fancy the value
var c = new MyFancyCollection<int>();
c.Fancy(42);

// Getting the values out
foreach (var v in c)
    Console.WriteLine(v);

Alternative Designs

No response

Risks

No response

Author: medo64
Assignees: -
Labels:

api-suggestion, area-System.Drawing, untriaged

Milestone: -

@safern safern removed the untriaged New issue has not been triaged by the area owner label Dec 6, 2021
@safern
Copy link
Member

safern commented Dec 6, 2021

Makes sense to add this overload to be consistent with what we have on other APIs, i.e Graphics.FillRectangle.

Marking it as api-ready-for-review to discuss it on the review meeting.

@safern safern added the api-ready-for-review API is ready for review, it is NOT ready for implementation label Dec 6, 2021
@terrajobst
Copy link
Member

terrajobst commented Dec 14, 2021

Video

  • Seems like a no-brainer
    • Are there any other? Seems like FillPie would be in the same category.
namespace System.Drawing.Common
{
    public partial class Graphics
    {
        public void DrawRectangle(Pen pen, RectangleF rect);
        public void FillPie(Brush brush, RectangleF rect, float startAngle, float sweepAngle);
    }
}

@terrajobst terrajobst added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Dec 14, 2021
@medo64
Copy link
Contributor Author

medo64 commented Dec 15, 2021

  • Seems like a no-brainer
    • Are there any other? Seems like FillPie would be in the same category.

Agreed; missed that one. I believe FillPie is the only one missing RectangleF but having Rectangle. There is a DrawIcon but there I believe having just Rectangle is appropriate.

@medo64
Copy link
Contributor Author

medo64 commented Dec 15, 2021

I did ask for pull request to be reopened (yes, I did things slightly out of order). Once that happens I will push commit for FillPie in addition to DrawRectangle that was already part of original pull request.

@deeprobin
Copy link
Contributor

Please tag this with "in pr" 😄 thank you

@safern safern added the in-pr There is an active PR which will close this issue when it is merged label Jan 3, 2022
@huoyaoyuan huoyaoyuan removed the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Mar 7, 2022
@jeffhandley
Copy link
Member

This was completed in #62385

@jeffhandley jeffhandley added this to the 7.0.0 milestone Mar 16, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Apr 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Drawing in-pr There is an active PR which will close this issue when it is merged
Projects
None yet
Development

No branches or pull requests

6 participants