I am embarrassed to say that I wasted an hour yesterday tracking this issue down in a test script I'm writing. It seems like an obvious thing to warn on, with fairly high correlation to errors. More exactly, we should warn when a drawArrays/drawElements call is made against a non-null framebuffer, and 'viewport has not been set accordingly'. I think the easiest way to help is to generally warn when the current viewport is larger than the FB we're rendering to.

Deliberate use of this is unusual, but possible. Cases where this might be desired include rendering to tiles from a very large viewport.

Drive-by review: Looks good to me. But really to provide feedback what you describe in the description, we should flag when a RTT is set and only clear the flag once a viewport is set. If the flag is set when hitting a draw call, emit a warning.

(In reply to Dan Glastonbury :djg :kamidphish from comment #3) > Drive-by review: Looks good to me. > > But really to provide feedback what you describe in the description, we > should flag when a RTT is set and only clear the flag once a viewport is > set. If the flag is set when hitting a draw call, emit a warning. The issue here is that they could set the viewport *before* binding a rendertarget. (stateful APIs \o/)

Comment on attachment 824948 [details] [diff] [review] warn-large-viewport Review of attachment 824948 [details] [diff] [review]: ----------------------------------------------------------------- We might need a second round if you agree with this: ::: content/canvas/src/WebGLContextVertices.cpp @@ +741,5 @@ > + mViewportHeight > rect->Height()) > + { > + GenerateWarning("Drawing to a destination rect smaller than the viewport rect."); > + } > + } This warning looks OK-ish, but as you mention, there are valid use cases where it will be generated. Another concern is that even when this warning is useful, being generated in every draw call, it will likely max out the max-warning-per-context limit immediately. Could you make it a one-off warning at least?

Alright, it only warns once per context.

Backed out for OSX bustage. https://hg.mozilla.org/integration/mozilla-inbound/rev/0fb5a87da513 https://tbpl.mozilla.org/php/getParsedLog.php?id=29988416&tree=Mozilla-Inbound

I needed to remove `inline` from a function to fix the build bustage. Thoughts?

Yes, I think what happened is that a function defined 'inline' in one translation unit that does not call it may be omitted (the assumption being that if another translation unit needs to call this inline function, it will take care of definining it; to that effect, 'inline' allows multiple definitions).

This issue is not resolved. The warning is still displayed, and it's a perfectly legitimate usecase to use a viewport smaller than the attached framebuffer (one such usecase is using it to implement mouse picking). Please remove the warning, because it's annoying.