Open Bug 934110 Opened 11 years ago Updated 2 years ago

Add an assertion to Matrix::TransformBounds that the rect's values are finite

Categories

(Core :: Graphics, defect)

x86
macOS
defect

Tracking

()

People

(Reporter: jwatt, Assigned: jwatt)

References

Details

I'd like to add a MOZ_ASSERT to Matrix::TransformBounds that the rect's values are finite. What do you think, Bas? Or maybe there's at least some warning macro that's suitable for use in Moz2D?
Flags: needinfo?(bas)
Blocks: 934183
I'm fine with that assert if you believe it will help :).
Flags: needinfo?(bas)
I'm undecided on what's better API: A) Require consumers to validate all input before passing it to Moz2D (in which case having Moz2D assert that all input is valid makes sense). B) Allow consumers to input non-finite values, junk paths, etc. and get junk metrics and other output (in which case asserts don't make sense). Since it's always possible that Moz2D will output junk for non-junk input, it seems like consumers need to check for junk output as necessary anyways. In that case (A) doesn't make much sense, on reflection. INVALID?
I actually prefer (A). The consumers may already be validating the input, and if we're also checking it inside of Moz2D, it's being done twice, and we're wasting some cycles. They could also be calling different Moz2D functions with the same input, so why validate it each time. If we leave it up to the consumer to do the right thing, it would be checked (at most) once.
Options A and B in comment 2 weren't consumer-checks-input vs. Moz2D-checks-input options. Rather they were consumer-checks-input vs. nobody-checks-input-but-consumer-checks-output. Or in practice, consumer-checks-input-and-output vs. nobody-checks-input-but-consumer-checks-output.
Got it. You're thinking of the overflow or some such situation, so why bother worrying about the input, if you can get bad stuff out even when input is good. To repeat your comment 2. How do we convince the caller to check the result?
(In reply to Milan Sreckovic [:milan] from comment #5) > You're thinking of the overflow or some such situation, Overflow, yes, but more likely trying to get the [stroke] bounds of a path created by |MoveTo(x,y); Close()| or |MoveTo(x,y); LineTo(x,y)| for the same {x,y}. Things like that. The SVG spec allows that in path commands in conjunction with square/round line caps as a way of creating little squares/circles in your path, so it's not uncommon. Unlike Cairo, Moz2D treats this inconsistently. On OS X in one of the cases I looked at it returns {inf,inf,0,0} for the bounds, which when transformed becomes {nan,nan,nan,nan}. > so why > bother worrying about the input, if you can get bad stuff out even when > input is good. Right. Which is why I said "INVALID?" there. It would be good to hear back from the Moz2D guys whether they agree that's what their API should be. > To repeat your comment 2. How do we convince the caller to > check the result? All we can do is note that sort of thing in the comments documenting the API, I think.
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.