Provide the Primitive screen bounds to shaders?

[kvark]

While investigating #1814, I found the real reason inner_rect had to be ignored in the #1820 workaround. Problem comes from the fact that our clipping is not in sync between Rust and GPU sides:

• Rust side takes the screen-aligned bounds into the prim_screen_rect, which is used for culling/visibility but not actually passed down to the shaders
• Shader side has the local primitive/clip rectangles but no the global rectangle, thus losing the screen-aligned bounds from sight

The workaround in #1820 is one way to address this. The comment needs to be changed, however, since it reads as if there is something wrong with the inner rect. It is untrue - this rectangle is totally correct. What is incorrect is skipping the axis-aligned clips that were already taken into account into the primitive screen bound, given that the shader doesn't know about this.

Another approach would be to add a rectangle to Primitive struct on the shader side, and then clip device_pos (before multiplication by the device-pixel ratio, I suppose) against it in write_transform_vertex of the vertex shader.

The performance costs are:

• doing the clamping in transformed VS - this is negligible
• actually providing the extra rectangle and passing it down the bus within PrimitiveGeometry - this may be concerning. I wonder if we can make it compile-time conditional based on the TRANSFORM define, in which case we should be safe.

The benefit is:

• creating/filling less clip masks, which I imagine can hit a few bad test cases if not avoided

[mrobinson]

Here's my summary of the issue:

1. The primitive boundaries passed to ClipChain processing code are already clipped by the screen boundaries of the clipping ClipScrollNode.
2. If we rejected clips based on these bounds, we might reject the clip which was itself responsible for created these ClipScrollNode screen boundaries. This happens especially when the content is clipped by a ClipScrollNode with a different transform.
3. The right thing to do here is to not reject the clip, since it does actually clip the node.
4. Additionally, it turns out that a lot of times the clip is just a rectangular clip aligned with the screen and the content is rotated. In this case, it might make sense to avoid turning a rectangular clip (aligned with the axis of the screen) into a mask, and instead do some kind of shader-based processing of the screen boundaries of the clip.

[mrobinson]

The mask is already limited by the primitive boundaries, so maybe the right thing to do here is to simply not render any pixels outside of the mask bounds?

[mrobinson]

Currently we support this situation by creating a an empty mask that clips to the screen boundaries of the clip chain.

webrender/webrender/src/prim_store.rs

Lines 1620 to 1637 in f1cecd7

	if clips.is_empty() {
	// If this item is in the root coordinate system, then
	// we know that the local_clip_rect in the clip node
	// will take care of applying this clip, so no need
	// for a mask.
	if prim_coordinate_system_id == CoordinateSystemId::root() {
	return true;
	}
	// If we have filtered all clips and the screen rect isn't any smaller, we can just
	// skip masking entirely.
	if combined_outer_rect == prim_screen_rect {
	return true;
	}
	// Otherwise we create an empty mask, but with an empty inner rect to avoid further
	// optimization of the empty mask.
	combined_inner_rect = DeviceIntRect::zero();
	}

We decided that this was a good tradeoff between quite of bit of added complexity in the shaders versus a very marginal performance improvement. Given that and the amount that the code has changed, perhaps it makes sense to close this bug and retake it if the performance of the current approach becomes an issue?

[glennw]

That sounds good, thanks!