Closed Bug 606722 Opened 12 years ago Closed 12 years ago

figure out what to do about animations to/from values of skew whose tangent is infinite


(Core :: CSS Parsing and Computation, defect, P2)




Tracking Status
blocking2.0 --- betaN+


(Reporter: dbaron, Assigned: dbaron)


(Keywords: css3)


(1 file)

In bug 598099 comment 6, bzbarsky wrote:
> This looks fine, for the most part.  The one worry is the skew distance
> calculation.  It can end up with diff == Infinity, I think, or worse yet diff
> being a NaN (e.g. when the float value is pi/2 for both v1 and v2, modulo
> rounding error).  Why did you decide to go with tan() here?

In bug 598099 comment 8, I wrote:
> I went with tan() because we also use tan() for interpolation of skew (since we
> animate it as though it was shear() rather than skew()).  I guess this is an
> issue for both distance and interpolation, though.

We need to figure out what to do about this.
blocking2.0: --- → betaN+
So _if_ I'm reading the spec correctly it just says to interpolate the skew angle itself, not the tangent (the latter is the matrix entry is all).  Is there an obvious problem with doing that?  I guess it'll act a little weird for mid-range skews, with the skew seeming to get faster as it goes?
Yeah, that is what the spec says.  I proposed changing it on www-style, though.
Ah, ok.  Let me think about this some more.  ;)
I think my inclination is to just do what the spec says rather than trying to improve it.  skew() probably should have been specified a shear(), but it's not.
Attached patch patchSplinter Review
This switches skew animation to be animation of angles.

Still waiting for full try results, but passed all tests on Windows and mochitest-plain-4 (the relevant one) on Linux opt.
Attachment #490796 - Flags: review?(bzbarsky)
Comment on attachment 490796 [details] [diff] [review]

Attachment #490796 - Flags: review?(bzbarsky) → review+
Closed: 12 years ago
Priority: -- → P2
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8
Pushed a followup to remove a now-unused variable (with r+a=dbaron in IRC):
You need to log in before you can comment on or make changes to this bug.