Runtime: API Proposal: Support operators * / on type Size types

[clairernovotny]

Size types (System.Drawing.Size, etc) represents a value that can be scaled up and down by multiplication and division.

A common scenario for this is scaling. Today, this involves taking the parts, doing the math individually, and then creating a new value.

Size, and related Size structs, should support * and / with both double and integer types.

This proposal should mirror https://github.com/dotnet/corefx/issues/7175

Proposed API

```C#
namespace System.Drawing
{
public partial struct Size
{
public Size operator(int left, Size right);
public Size operator(Size left, int right);
public Size operator/(Size left, int right);

    public SizeF operator *(float left, Size right);
    public SizeF operator *(Size left, float right);
    public SizeF operator /(Size left, float right);
}
public partial struct SizeF
{
    public SizeF operator*(float left, SizeF right);
    public SizeF operator*(SizeF left, float right);
    public SizeF operator/(SizeF left, float right);
}

}
```

[terrajobst]

Hmm, I'm not sure it's worth it Oren. It would take years to get into .NET Framework! 😛

That scenario makes sense. Here is what I think we should be doing:

```C#
namespace System.Drawing
{
public partial struct Size
{
public Size operator(int left, Size right);
public Size operator(Size left, int right);
public Size operator/(Size left, int right);
}
public partial struct SizeF
{
public SizeF operator(float left, SizeF right);
public SizeF operator(SizeF left, float right);
public SizeF operator/(SizeF left, float right);
}
}

That allows you do that like this:

```C#
Size oldSize = ...
Size newSize = Size.Round(oldSize * 1.5);

[clairernovotny]

Are there similar structs in System.Windows.* that need this (WPF?)

[karelz]

@alexperovich does it sound reasonable? If yes, please mark it api-ready-for-review.
@onovotny needs a proof we can do API reviews fast 😝 (at least on Core if not Desktop)

System.Windows.* does not live in CoreFX, so we cannot influence it from here :(

[alexperovich]

This is reasonable.

[mellinoe]

This proposal isn't really complete... the operators don't have return types 😄. @terrajobst I'm assuming you meant for them all to return SizeF ? Otherwise your second example wouldn't really make sense (Round doesn't work on Size, because it's an integral type).

[alexperovich]

I added return types to the proposal. I think the Size operators should return Size and the SizeF operators should return SizeF.

[mellinoe]

The original post mentions this:

Size, and related Size structs, should support * and / with both double and integer types.

@onovotny How does this latest version look? How would you expect a Size * double method to behave?

[clairernovotny]

@mellinoe I'd expect Size * double to return SizeF and Size / int to return SizeF. Then the user can use Size.Round to get a Size if that's the needed result (most likely).

[tannergooding]

I don't think I'd ever expect Size * double or Size / int to return SizeF.

I would expect Size * float or Size * double to not exist and for Size / int to return Size.

[clairernovotny]

Size * float or double is the most common case for scaling. In winforms, it's var scale = (float)DeviceDpi / 96;. Then use that to change Size values used everywhere.

[tannergooding]

I understand its common to scale using floating-point values :)

I was just indicating, from an API perspective, I would not expect that Size / int returns SizeF or that Size * double existed (and if it did, I would expect it to return Size, not SizeF).

If I'm working with Size, I would expect that I continue working with Size (for inputs and outputs) unless I explicitly cast somewhere.

[tannergooding]

That being said, I can at least see the argument for Size * float returning SizeF, since it functions in a similar manner to int * float returning a float.

Although, Size / int I'm still not sold on, since int / int is an int.

[clairernovotny]

I agree on Size / int returning Size. While technically it'd be SizeF, that behavior isn't consistent with how everything else works.

[alexperovich]

Actually, Size is implicitly convertible to SizeF. So with the current proposal Size * float would return a SizeF. And Size * int would be Size

[tannergooding]

I think I've convinced myself the APIs make "sense" if they followed the existing operator logic for int and float.

So
```C#
namespace System.Drawing
{
public partial struct Size
{
public Size operator(int left, Size right);
public Size operator(Size left, int right);
public Size operator/(Size left, int right);

    public SizeF operator *(float left, Size right);
    public SizeF operator *(Size left, float right);
    public SizeF operator /(Size left, float right);
}
public partial struct SizeF
{
    public SizeF operator*(float left, SizeF right);
    public SizeF operator*(SizeF left, float right);
    public SizeF operator/(SizeF left, float right);
}

}
```

I didn't include the int * SizeF, SizeF * int, and SizeF / int operators, since the int needs to be a float anyways and int is implicitly converted to float.

I don't think operators which take double should exist. The results have to be downcasted in the end anyways, and single pre-cast down to float is less expensive than multiple explicit downcasts. There is also no implicit behavior for double->float today (its all explicit).

[terrajobst]

Yep, after talking for 15 minutes we convinced ourselves that this API shape indeed makes the most sense:

```C#
namespace System.Drawing
{
public partial struct Size
{
public Size operator(int left, Size right);
public Size operator(Size left, int right);
public Size operator/(Size left, int right);

    public SizeF operator *(float left, Size right);
    public SizeF operator *(Size left, float right);
    public SizeF operator /(Size left, float right);
}
public partial struct SizeF
{
    public SizeF operator*(float left, SizeF right);
    public SizeF operator*(SizeF left, float right);
    public SizeF operator/(SizeF left, float right);
}

}
```

[WinCPP]

@karelz something that I can work on...?

[karelz]

Sure, just be aware that we might need to postpone taking the change after May 10 - see https://github.com/dotnet/corefx/issues/17619#issuecomment-296872132 for details.

[WinCPP]

@karelz yup. won't nag folks busy with 2.0 ;-)

[karelz]

Thanks, appreciate that @WinCPP!

[jnm2]

I'm really motivated get this in 2.0. Can I help?

[karelz]

This will likely NOT get in 2.0 - see my comment https://github.com/dotnet/corefx/issues/18017#issuecomment-297179050
We are sort-of locked down for 2.0 API surface.

[jnm2]

@karelz When you say "likely"... is that just a polite way of saying no? I'd hate for someone to decide they could squeeze it in at the last minute, but too late for @WinCPP or me to PR.

[karelz]

By "likely" in this context I meant that I am not the final decision maker.
And also that it depends on how big and scary the change is ...

Personally, I would leave it to post-2.0. Honestly, having 10 more APIs is not going to move the needle. Even adding 100 more APIs is probably not going to change much for 2.0. Of course, unless one of them is huge pain to many developers ;).
What's needed most for .NET/ .NET Core platform to be successful is IMO to have the right technologies, protocols, specs/RFCs, ease-of-use of conceptually hard APIs, compatibility packages with Desktop, and high performance. All the hard stuff, which takes time ...

[WinCPP]

@onovotny in case of Size * int, I believe we are ok with integer overflow on same lines as in case of Add (link)...? Thanks!

[WinCPP]

@onovotny gentle ping... please see above... held off because folks were busy with 2.0... thanks! 😃

[clairernovotny]

@WinCPP I think so, but that may be a better question for @terrajobst

[karelz]

If you think we need adjustment to the API proposal, please paste it here and I will mark it again for API review once you and area expert @mellinoe agree on the shape.

PS: @terrajobst is IMO swamped with GH notifications as he doesn't often answer ;)

[clairernovotny]

@karelz I don't think this changes the API shape, it's more about how to handle integer overflows in the implementation.

[WinCPP]

I think I will go by the existing behavior i.e. overflows are allowed and we don't precheck and throw exceptions as is done for TimeSpan issue dotnet/runtime#16802.

Rational being,

1. Throwing exceptions only in operator * and operator / would be inconsistent with behavior of other two operators operator + and operator - on the same type.
2. Changing behavior of existing operators as well to throw exceptions will be a behavioral break for existing code.

For now, I have raised the PR with the code. Please review and let me know.

[karelz]

I let @mellinoe decide if he wants to run it by API review ...

Thanks for explanation @onovotny! I usually go through quite a lot of bugs, so I don't scale deep dive into each of them :( ... please let me know if you ever feel I am becoming more an obstacle than someone who mostly helps ;)