Conversation
| Rect bounds = Rect::MakeOriginSize(params_.center, | ||
| Size(params_.size.x, params_.size.y)); |
There was a problem hiding this comment.
I think currently this is just the positive quadrant of the bounds, rather than the full bounds. For the full bounds, should origin be center - size, and size be params.size * 2?
There was a problem hiding this comment.
Ooops, you're right. D'oh! Or add *2 to the constructed Size object here.
Also, this points out that by storing these "size" values as Points we are fighting our CPU code a bit, but the values in the FragInfo have to be GPU types which are vec2/Point.
There was a problem hiding this comment.
I was thinking it was CenterSize, but double D'oh!
| if (!transform.IsTranslationScaleOnly()) { | ||
| return false; | ||
| } | ||
| return GetExpandedDeviceBounds(transform, true).Contains(rect); |
There was a problem hiding this comment.
Do we really need to inset by AA here?
I think the regular base bounds would be considered fully inside the coverage area. I think AA padding doesn't shrink the coverage area in any way. So we can just use the transformed base bounds, and wouldn't need to account for AA here. Is that correct, or am I misunderstanding something?
There was a problem hiding this comment.
Never mind. Thinking about it some more, I see how the AA is smudging the edge of the base bounds in both the positive and negative direction. I think you're correct here.
There was a problem hiding this comment.
And the things it covers are smudging their output by the AA fringe as well. This could get ugly if there are cases where we look for coverage for precise boundaries. I'd have to look at how this method is used (and its doc comments could be more explicit about a lot of this).
| transform.GetMaxBasisLengthXY()); | ||
| } | ||
|
|
||
| GeometryResult UberSDFGeometry::GetPositionBuffer( |
There was a problem hiding this comment.
So GetPositionBuffer returns results in local (untransformed) space. And GetCoverage/CoversArea works in device (transformed) space. I think this is not very obvious, and should be explicitly stated in geometry.h.
There was a problem hiding this comment.
I only learned this by looking at other implementations...
A proposal to simplify UberSDFGeometry.