[PATCH 8/9] quartz/vmr9: Use assignment instead of CopyRect

[Gabriel Ivăncescu]

On 12/30/18 00:27, Michael Stefaniuc wrote:
> On 12/29/18 12:24 PM, Gabriel Ivăncescu wrote:
>> On 12/28/18 23:20, Michael Stefaniuc wrote:
>>> I'm assuming those methods will do argument validation with proper error
>>> code should the RECTs be NULL. Needs tests of course. But that's the
>>> reason I didn't replace those when I did my CopyRect() removals back in
>>> the day.
>>>
>>> Also we are in change freeze and pure janitorial work is kinda forbidden
>>> during that; especially in the later rc versions.
>>>
>>
>> Sorry, I had assumed that it's mostly janitorial work that is acceptable
>> during code freeze, that's why I sent them, my bad :-)
>>
>> To be honest, I agree with Nikolay's point about most of these remaining
>> CopyRect (except those not requiring NULL checks, like the combobox
>> CopyRect removal).
>>
>> But in such case, they should have inline helpers to be consistent with
>> the others. Hopefully it's reconsidered after code freeze (the original
>> inline patch), though I will split it into PtInRect and CopyRect
>> separately, at least for consistency.
> Really, there is no need for consistency here. The inline versions were
> introduced to avoid the addition of a cross dll function call when the
> open coded stuff was replaced.
>
> bye
> 	michael
>

But the cross dll function call happens in this case also. I understand 
why they were introduced, but it doesn't mean that should be the only 
reason they can be introduced.

For example, if this function started with open coding and then got 
replaced with CopyRect, I assume you would add the inline version, 
correct? Even though the code in the end would be identical to now.

That's why it seems so arbitrary and confusing for someone who skims 
through the code. I understand why you did not add them yourself, but 
that doesn't mean they shouldn't be added, at least for consistency's 
sake. IMO.

Clearly it breaks nothing since if they were to replace an open-coded 
variant, they would *also* affect any other code that currently uses 
CopyRect, and obviously in that case I'm sure it would be accepted 
immediately...