924573 - Consistently deal with canvas width or height set to 0

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

[Milan Sreckovic [:milan] (needinfo for best results)]

See bug 916128 as well.  0 width and height is valid in the standard (allows non-negative), but we don't handle it consistently, ending up with cases where canvas and context have non-matching sizes.

Comment 1

[Milan Sreckovic [:milan] (needinfo for best results)]

https://tbpl.mozilla.org/?tree=Try&rev=cd480adc74a1 before I go further.

Comment 2

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: Only change the zero height or weight to 1, not the other parameter as well. r=bas

Comment 3

[Bas Schouten (:bas.schouten)]

Comment on attachment 815097 [details] [diff] [review]
Only change the zero height or weight to 1, not the other parameter as well.  r=bas

Review of attachment 815097 [details] [diff] [review]:
-----------------------------------------------------------------

Hrm, interesting, is this the behavior we want really? Theoretically the discrepancy between the requested and the given surface size is much larger with this method than just always making it 1. (Since the requested surface size is 0). I'd like to understand what makes the discrepancy of 1 and 0 tolerable while the other is not? Perhaps, since this is an implementation detail, we should have an internal 1x1 surface but simply report the size back differently?

Comment 4

[Milan Sreckovic [:milan] (needinfo for best results)]

It is a bit of a mess.  The standard does allow zero (non-negative values are OK), so we have tests that set it to zero and want us to tell about it.  But in the context, we just quietly reset it to 1, because we don't want to handle the zero case.  I couldn't find the explanation in the code why we chose 1x1 instead of 1xN, when 0xN is given as the size. So, it seems like 0 == 1 is acceptable.
I can easily leave the original 1x1 code and just change the assert to not worry about the size comparison when one of the dimensions is zero (or, more to the point, assert that it's 1x1, I guess), but it is a special case, and the full solution, which would probably be to make the contexts handle zero internally correctly is likely not worth the effort.
What do you think?

Comment 5

[Bas Schouten (:bas.schouten)]

Comment on attachment 815097 [details] [diff] [review]
Only change the zero height or weight to 1, not the other parameter as well.  r=bas

Review of attachment 815097 [details] [diff] [review]:
-----------------------------------------------------------------

Let's just do this, it's not worth investing additional time in as you say. Let's fix it properly at some point.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/877210776b71

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/877210776b71