Bug 93523 - Why doesn't RotateTransformOperation::blend() check for |this| being axis-aligned too?
Summary: Why doesn't RotateTransformOperation::blend() check for |this| being axis-ali...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-08-08 14:22 PDT by Simon Fraser (smfr)
Modified: 2023-10-25 14:26 PDT (History)
11 users (show)

See Also:


Attachments
Shows error on Chrome (555 bytes, text/plain)
2012-08-08 18:52 PDT, Dirk Schulze
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2012-08-08 14:22:48 PDT
In the     // Optimize for single axis rotation clause, why does it only check if 'fromOp' is axis-aligned. Shouldn't it also check that |this| is axis-aligned in the same way?
Comment 1 Dirk Schulze 2012-08-08 18:52:51 PDT
Created attachment 157366 [details]
Shows error on Chrome

I wrote a simple test case. It is indeed a bug not to check the resulting rotation as well. Actually it does not affect Safari, since it seems to use the logic of CoreAnimation. But you can see the problem in Chrome. The element just gets rotated by the y-axis of the initial transform and does not animate around the x-axis as well. It jumps to the correct result after the animation.

I wonder why we just optimize for vectors of the length of 1? If the vectors go in the same direction, the length doesn't matter. So it should be more like:

((fromOp->m_x == m_x) == 0 &&
(fromOp->m_y == m_y) == 0 &&
(fromOp->m_z == m_z) > 0)

for the z-axis and similar for every axis. It should also be possible to use the fast track if both vectors are negative.
Comment 2 vollick 2012-08-14 07:29:54 PDT
Could we do something like this?

// Optimize for single axis rotation
if (!fromOp || hasSameAxis(*fromOp)) {
    double fromAngle = fromOp ? fromOp->m_angle : 0;
    return RotateTransformOperation::create(m_x, m_y, m_z,
        WebCore::blend(fromAngle, m_angle, progress), m_type);
}

where,

bool RotateTransformOperation::hasSameAxis(const RotateTransformOperation& other)
{
    double length = sqrt(m_x * m_x + m_y * m_y + m_z * m_z);
    double otherLength = sqrt(other.m_x * other.m_x + other.m_y * other.m_y + other.m_z * other.m_z);

    if (length <= EPSILON || otherLength <= EPSILON)
        return false;

    double dot = other.m_x * m_x + other.m_y * m_y + other.m_z * m_z;
    double error = fabs(1.0 - dot / (length * otherLength));
    return error < EPSILON;
}
Comment 3 Ahmad Saleem 2023-10-25 14:26:44 PDT
Changed test in JSFiddle (not removed -webkit-): https://jsfiddle.net/9Lonmyxr/

WebKit ToT and Chrome Canary 120 seems to work similar or I am missing anything?