This is an archived snapshot of W3C's public bugzilla bug tracker, decommissioned in April 2019. Please see the home page for more details.

Bug 10930 - CanvasPixelArray out of range behavior needs clarification
Summary: CanvasPixelArray out of range behavior needs clarification
Status: VERIFIED FIXED
Alias: None
Product: HTML WG
Classification: Unclassified
Component: LC1 HTML Canvas 2D Context (show other bugs)
Version: unspecified
Hardware: All All
: P2 normal
Target Milestone: ---
Assignee: Ian 'Hixie' Hickson
QA Contact: HTML WG Bugzilla archive list
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-10-01 01:09 UTC by Jatinder Mann [MSFT]
Modified: 2011-08-06 13:15 UTC (History)
12 users (show)

See Also:


Attachments

Description Jatinder Mann [MSFT] 2010-10-01 01:09:23 UTC
The spec is clear that the CanvasPixelArray contains image data in the range of 0..255, representing the 8 bit value for that component. However, the spec is not clear on what the expected behavior should be when a value greater or less than the range of 0..255 is assigned to the array.

The spec should be updated to include the following text:

"When a CanvasPixelArray object is indexed to modify an indexed property index, with value value, the value of the component occupying the position index, in the array must be set to value. If value is less than 0, it must be clamped to zero. If value is more than 255, it must be clamped to 255."

Firefox, Chrome and Safari follow the behavior described in the proposed text. This behavior is more inline with a previous version of the spec.

Rounding to the limits of 0 or 255 when out of range seems to be more logical to a web developer than wrapping the value. 

http://lists.w3.org/Archives/Public/public-canvas-api/2010AprJun/0001.html
Comment 1 Tab Atkins Jr. 2010-10-01 03:27:23 UTC
To be clear, the "wrapping the value" behavior the previous comment refers to is the behavior required by WebIDL's octet type.

I agree that clamping rather than wrapping is much more intuitive, and matches current implementations.
Comment 2 Boris Zbarsky 2010-10-01 03:31:58 UTC
Er, yeah.  Webidl for octet also introduces a bias towards zero if your numbers actually fall into the 0-255 range (due to the use of floor()).  The spec here used to specify quite different clamping and rounding behavior; why did that get removed?
Comment 3 Silvia Pfeiffer 2010-10-01 03:54:19 UTC
Also note that Opera wraps the value, which confused me highly recently.
Comment 4 Philip Taylor 2010-10-01 10:54:37 UTC
(In reply to comment #2)
> The spec here
> used to specify quite different clamping and rounding behavior; why did that
> get removed?

http://lists.w3.org/Archives/Public/public-canvas-api/2010AprJun/0002.html

  "This text was removed because it would never have been invoked, since Web IDL changes the value before the method is invoked, so you can't actually invoke the method with out-of-range values. So it was rather confusing!"
Comment 5 Boris Zbarsky 2010-10-01 15:51:05 UTC
But the right fix there would have been to not use the |octet| webIDL type, not to change the behavior....
Comment 6 Ian 'Hixie' Hickson 2010-10-04 23:12:15 UTC
Octet is the right type here, since the value is specifically intended to be one byte.

Reassigning to the WebIDL component so that heycam can fix WebIDL's rounding behaviour to match what browsers do here.
Comment 7 Cameron McCormack 2010-10-04 23:16:41 UTC
I'll have to look in to what other octet-using APIs (are there any?) do, too.  Hopefully it's not the case that some clamp while others do a mod.
Comment 8 Kenneth Russell 2010-10-04 23:27:23 UTC
A similar issue was discovered while designing the Typed Array specification (https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/doc/spec/TypedArray-spec.html). The original clamping behavior of CanvasPixelArray, rounding values less than 0 to 0 and values greater than 255 to 255, leads to extremely inefficient machine code for the set operation. It appears that the reason that ECMA-262's ToInt32 and similar operations are specified the way they are is that they are efficiently implementable with machine instructions. The same holds for Web IDL's octet conversion rules for ECMAScript.

The Typed Array specification follows the ECMA-262 and Web IDL conventions, for what it's worth.
Comment 9 Boris Zbarsky 2010-10-05 01:30:57 UTC
Extremely inefficient compared to what?  Compared to a single C cast to (unsigned char)?  Sure.  Note that you can't do that anyway in general because it might be NaN, etc, so you have to check for those.

But more importantly, this behavior is unstable.  Floating-point rounding errors can suddenly send your white "almost 256" value to black.  That seems _very_ undesirable.
Comment 10 Kenneth Russell 2010-10-05 01:52:52 UTC
(In reply to comment #9)
> Extremely inefficient compared to what?  Compared to a single C cast to
> (unsigned char)?  Sure.  Note that you can't do that anyway in general because
> it might be NaN, etc, so you have to check for those.

Inefficient compared to C casts, the Web IDL conversion rules, and the definitions of ToInt32 and similar conversions in the ECMA-262 specification itself.

Yes, an explicit NaN test is still required, but that is a small cost compared to the clamping behavior that was previously specified by CanvasPixelArray, which required differentiating even between positive and negative infinity.

> But more importantly, this behavior is unstable.  Floating-point rounding
> errors can suddenly send your white "almost 256" value to black.  That seems
> _very_ undesirable.

256 is an out-of-range value for CanvasPixelArray. If the value were nearly, but not quite, 255, which is what correct code should be producing for white, then any rounding mode (round to zero or round to nearest) will produce reasonable results.
Comment 11 Boris Zbarsky 2010-10-05 02:01:34 UTC
> which is what correct code should be producing for white

Plenty of code out there wants to take 128 and double it to go from gray to white, in my experience.  And again, the proposal of using floor() will 0-bias even in the absence of this issue.

In any case, a number of engines implement this stuff and the cost of the code on the sets there doesn't seem to be holding them back much performance-wise.  (In fact, it's minuscule in every single profile of anything using imageData I have ever done compared to various other things that need to happen.)  So I really don't buy the efficiency argument; the mod behavior would in fact be faster than clamp+round, but it's not a hotspot for this stuff.

One other note on performance in case someone does think it matters; in practice, code will in fact want the clamping behavior.  So if it's not done in imageData, it will simply move out into boilerplate JS which will make things even slower.

(All this quite apart from the "compatibility with deployed content" issue...  I know I've seen some canvas stuff rendering incorrectly in Opera due to its different behavior here.)
Comment 12 Jatinder Mann [MSFT] 2010-10-06 22:22:55 UTC
We are planning to change the IE9 implementation to the prior clamping approach for compatibility with other implementations and any existing web content that expects the previous behavior. I recommend we update the spec.
Comment 13 Cameron McCormack 2011-05-23 03:19:52 UTC
Given Opera's the only one that doesn't clamp now, I think we should just make the CanvasPixelArray special operations clamp their arguments.  I don't see a problem with wrapping in general.  Unless there strong objections, I'll add a [Clamp] extended attribute that overrides the default JS to IDL conversion behaviour, which Ian can then use in the HTML spec.
Comment 14 Cameron McCormack 2011-05-31 03:54:31 UTC
I've added [Clamp] now:

http://dev.w3.org/2006/webapi/WebIDL/#Clamp
http://dev.w3.org/2006/webapi/WebIDL/#es-byte
http://dev.w3.org/2006/webapi/WebIDL/#es-octet etc.

Reassigning to Hixie for HTML treatment.
Comment 15 Michael[tm] Smith 2011-08-04 05:03:43 UTC
mass-move component to LC1
Comment 16 Ian 'Hixie' Hickson 2011-08-06 03:52:02 UTC
EDITOR'S RESPONSE: This is an Editor's Response to your comment. If you are satisfied with this response, please change the state of this bug to CLOSED. If you have additional information and would like the editor to reconsider, please reopen this bug. If you would like to escalate the issue to the full HTML Working Group, please add the TrackerRequest keyword to this bug, and suggest title and text for the tracker issue; or you may create a tracker issue yourself, if you are able to do so. For more details, see this document:
   http://dev.w3.org/html5/decision-policy/decision-policy.html

Status: Accepted
Change Description: see diff given below
Rationale: Concurred with reporter's comments.
Comment 17 contributor 2011-08-06 03:52:23 UTC
Checked in as WHATWG revision r6381.
Check-in comment: Use [Clamp] to return this part of the spec to the real world.
http://html5.org/tools/web-apps-tracker?from=6380&to=6381
Comment 18 contributor 2011-08-06 03:54:12 UTC
Checked in as WHATWG revision r6382.
Check-in comment: Update previous checkin to actually be in the real WebIDL world instead of some crazy meaningless nonsense. Oops.
http://html5.org/tools/web-apps-tracker?from=6381&to=6382