Revert change from SVGRect/SVGPoint/SVGMatrix to DOM equivalents

[longsonr]

https://www.w3.org/TR/SVG2/types.html#SVGDOMOverview

SVG 1.1. had types SVGRect, SVGPoint and SVGMatrix. In SVG2 these have been replaced by DOMRect, DOMPoint and DOMMatrix (or in some cases DOMRectReadOnly or DOMRectInit etc)

Unfortunately I don't think this is going to work. viewBox.animVal and viewBox.baseVal are live, if you change them the viewBox attribute changes. That means you need to have something derived from DOMRect/DOMRectReadOnly to model that and have DOMRect and DOMRectReadOnly methods be virtual.

The same is true of various DOMPoint, DOMMatrix things in the SVG2 spec too.

No browser has dropped SVGPoint etc and I suspect the above is the reason why. I don't think I can make the change in Firefox for instance because of performance concerns over having virtual functions.

So I'd like to propose that the specification reverts to SVGPoint, SVGMatrix and SVGRect except for inputs to methods (e.g. IsPointInFill), which can stay as DOMPointInit, DOMMatrixInit and DOMPointInit because those dictionaries merely want something with the appropriate properties so they will take an SVGPoint, SVGMatrix or SVGRect.

Note that some methods still don't take Init input argument e.g. getEnclosureList but perhaps I should raise that as a separate issue.

[AmeliaBR]

To clarify:

You're saying that SVGPoint, etc, should be reintroduced, but as subclasses of DOMPoint, etc. The subclass wouldn't have any extra author-exposed functionality, but would have an internal slot that represented the association to an element/attribute, so they can be live reflections of the attribute?

[longsonr]

No SVGPoint should be reintroduced exactly as it was in SVG 1.1 i.e not a subclass of DOMPoint.

[AmeliaBR]

I guess I don't understand, then: What is the obstacle to extending DOMPoint, etc, to make it reflect an attribute value? Is it just a performance issue, that you currently have the components (x, y, etc.) implemented as simple attributes of the object, so you don't want to turn them into getters/setters?

The whole purpose of the DOM geometry classes was to replace the matching SVG classes, but make them usable more broadly. It's disappointing if they were implemented in a way that doesn't make that possible. I really don't like the idea of maintaining parallel but independent sets of geometry classes.

[longsonr]

https://dxr.mozilla.org/mozilla-central/source/dom/base/DOMPoint.h

In Firefox's implementation DOMPoint is final and none of the methods in either DOMPoint or DOMPointReadOnly are virtual. Making them virtual would adversely affect html perf and is therefore a problem. Same for DOMRect/DOMRectReadOnly

DOMPoint/DOMRect are clearly not things that are designed to be live. They are fine for the result of GetBBox but not for viewBox.baseVal or viewBox.animVal.

[AmeliaBR]

Ok, so let's get back to your proposal: How would SVGPoint/DOMPoint and SVGRect/DOMRect be related? The SVG classes would be wrappers for an internal DOM geometry type, exposing the internal object's components using getters/setters that also have the side effects necessary for attribute reflection?

That seems do-able, if not ideal. How much are these objects used currently in built-in APIs other than SVG?

We'd need to go through all the relevant methods to see which should be accepting or returning which type. I think their use as parameters, at least, are already covered by the *Init type definitions.

We might also want to make a note that the SVGSVGElement's .createSVGPoint(), etc., methods are deprecated (because the extra overhead of the getter/setter etc. would not have any value for a detached geometry object that isn't associated with a reflected attribute). Instead, we could recommend that the DOM* methods and their constructors be used for calculations in author scripts.

---

I'd love to get some feedback from other implementers about how the different options compare in your codebases!

[longsonr]

SVGPoint and DOMPoint would not be related. Neither would SVGRect and DOMRect. Their attributes would share common naming so that they could both be passed to things that take DOMPointInit or DOMRectInit. Same with SVGMatrix/DOMMatrix.

If we have everything taking DOMPointInit, DOMRectInit etc then we don't need createSVGPoint or createSVGRect.

[fsoder]

I can acknowledge that this transition has not gone as smooth as some may have hoped. I wouldn't want to acknowledge the specific problem statement however - I'd fully expect this transition to involve additions of state to DOMPoint and similar ("read only" and "associated" for example). Comparing options here leaves us in the old (potentially unfair) comparison of "no work" (status quo) and "non-trivial (albeit not difficult) work".

[dirkschulze]

@fsoder agreed.

@longsonr SVGPoint and SVGRect are aliases fro DOMPoint and DOMRect according to the Geometry Interfaces spec. So replacing the former with the latter (or DOM*Init in some cases) were more or less a formality. Changing them back wouldn't make a difference.

While baseVal and animVal certainly are in use, the use counter is very likely very low. Is there anything we could do in SVG2 or Geometry Interfaces to reduce the burden that you see? I thin I remember that Firefox already did backwards-incompatible changes to animVal because low usage numbers allowed it. Could you clarify that?

[longsonr]

I mean change them back to SVG 1.1 where they are unrelated to DOMPoint and DOMRect.

We've not made any backwards-incompatible changes to animVal.

We have other liveness problems with DOM types

• currentTranslate as a DOMPoint
• SVGAnimatedPoints (e.g. SVGPolylineElement) having SVGPoints
• DOMMatrix in transforms

[dirkschulze]

@longsonr I understood the request but

1. It requires the actual change to be done in Geometry Interfaces first since that spec defines SVG interfaces as aliases to the corresponding DOM interfaces.
2. I actually think it is a step backwards. We wanted to unify the web world with Geometry Interfaces and pull out SVG of its silo isolated from the rest of the web world. We got strong support from web developers for that change.

So before we discuss undoing the changes I'd rather like to hear proposals what we can do to make the transition less painful.

[AmeliaBR]

I'd also like to hear more feedback & proposals from other implementation teams.

If no one has plans to update their implementations to replace usage of SVGMatrix, etc. with DOMMatrix, then we really should be changing the spec to match reality.

For reference, here are the tracking bugs for making the SVG interfaces aliases to the DOM equivalents:

• Chromium 709001
• Mozilla 1563444 (tracking bug), and 1439685 about SVGRect in particular but with lots of discussion of implementation issues
• WebKit: SVGMatrix, SVGRect, SVGPoint

If the final conclusion is that browsers do not want to update the DOM geometry interfaces to support attribute reflection, then we need to go through the full SVG IDL and identify which uses of the geometry interfaces need a version with attribute reflection and which can still be replaced by the DOM equivalent. E.g., the value returned by getCTM() will never be associated with an attribute, so it could return a DOMMatrix without any changes to the DOMMatrix definitions.

I'd also like any suggestions about how we could make the difference between a DOM* geometry interface and its SVG equivalent as transparent as possible to authors.

[longsonr]

@AmeliaBR Bug 1439685 is where I was actually trying to do this.

[AmeliaBR]

Thanks Robert, added it to the list.

[saschanaz]

which can still be replaced by the DOM equivalent

Could the partial replacement without LegacyWindowAlias potentially cause compatibility problem? Code may expect instanceof SVGRect check to pass, but returning a DOMRect without aliasing will not pass the check.

[longsonr]

@saschanaz There's still aliasing, see https://github.com/w3c/svgwg/issues/706#issuecomment-506639713

[saschanaz]

@longsonr Yes, but the suggested rollback will need to remove that aliasing.

[longsonr]

@saschanaz There's no browser implementation rollback here, just the spec potentially changing to match reality.

[saschanaz]

@longsonr I mean if we rollback the spec as you suggested and still return DOM* for some non-reflecting cases as @ameliabr suggested.

[css-meeting-bot]

The SVG Working Group just discussed Revert change from SVGRect/SVGPoint/SVGMatrix to DOM equivalents.

The full IRC log of that discussion

<AmeliaBR> Topic: Revert change from SVGRect/SVGPoint/SVGMatrix to DOM equivalents
<myles> GitHub: https://github.com/w3c/svgwg/issues/706
<myles> krit: there are multiple problems. For context, SVG 1.1 had their own geometry information. SVGRect, SVGPoint, SVGMatrix, etc. Then we combined them with CSS objects. There was a CSS Matrix, CSS Rect, etc. This would be DOMPoint, DOMMatrix, etc.
<myles> krit: And replace all the types with the new types.
<myles> krit: Robert from Firefox sees issues with this approach and requests that we change it back to SVG*** from DOM***
<myles> AmeliaBR: The issue is the implementations in Firefox and maybe other implementations, the DOM classes were done in a lightweight way that the individual values within the object are implemented as literal values in the underlying C code, so they can't be used for getters and setters that have side effects, which are necessary if you're going to do attribute reflection
<sairus_> on phone :)
<myles> AmeliaBR: as I understand it, the concern is that if they change that implementation so that attribute reflection would be supported, it would reduce the overall performance of the geometry interfaces in other contexts.
<myles> AmeliaBR: I didn't add this to the agenda expecting a decision. We need to get more feedback from more implementors about looking at what is in your code, what are the blockers for combining the two sets of interfaces, and if there is a decision that nobody has an intention to combine these interfaces, then what's the best way to go forward?
<myles> krit: Or, rephrasing: We have those properties in the SVG DOM, which means that any change that comes from anywhere are reflected everywhere.
<myles> krit: Usually a property that returns an object, it has no bindings to the original value that created the API
<myles> krit: But, instead of a new object, it gives you an object that references the internal value
<myles> krit: that's waht we call "live-ness"
<AmeliaBR> `el.viewBox.baseVal` returns an SVGRect/DOMRect which remains live-linked to the `viewBox` attribute.
<myles> AmeliaBR: The viewbox attribute is reflected as an object. If you change that object, the attribute updates. But, if you change the attribute somewhere else, the object that you're holding a reference to needs to update to. So, internal to the implementation of that object, it needs a poitner back to the element. Instead of storing the values in a permanent way, it needs to have getters and setters that check the current values on the DOM element.
<myles> AmeliaBR: The other use case for these geometry properties as something you can do math with in a way that, you call a method and it returns an object htat has X, Y, width, and heigh tproperties, and you can do transforms on it
<myles> AmeliaBR: And if you do math on it, it doesn't have a conection to the original element.
<myles> AmeliaBR: So, if you make it live, it gets slower
<myles> krit: myles, can you ask shallawa for advice?
<myles> myles: yes
<myles> AmeliaBR: We need feedback from implementors before we can proceed.
<myles> krit: Let's wait for the WebKit investigation
<myles> AmeliaBR: the SVG*** interfaces are the live, previously impelmented interfaces, though they also work in a detached way if you want. The DOM ones where defined to replace the SVG ones but the implementations that went ahead only focused on the detached use cases, and don't support the live use cases.
<myles> krit: All the SVG ones are live.
<myles> SVGAElement implements two conflicting interfaces

[shallawa]

First I want to say that I support the current SVG 2 plans regarding using DOM geometry interfaces. I do not like putting burden on the web developer by exposing many similar data structures with different names and with minor differences, e.g. DOMPointInit, DOMPointReadOnly, DOMPoint and SVGPoint. If we can get rid of one of them, let's do it.

I would suggest the following if we want to have the DOM geometry interfaces alive. These interfaces should be explicit about what object owns them. A new interface should be defined for the "data owner" or the "object parent" or whatever, say we name it DOMDataOwner. The DOMDataOwner is not necessarily a Node or Element. DOMPoint, DOMRect and DOMMatrix should have a pointer to DOMDataOwner. This pointer can be null which means the object is detached. All the objects will be detached except the ones that are attached to SVGElements or SVG DOMDataOwner. So there should not be a performance issue for non the SVG case.

In C++, DOMDataOwner can be defined like this:

class DOMDataOwner {
public:
    virtual DOMDataOwner* owner() const { return nullptr; }
    virtual void commitChange()
    {
        if (!owner())
            return;
        owner()->commitChange();
    }
};

Let's consider this JavaScript code as an example of when we need to need to propagate a change through multiple DOMDataOwners to be committed to the Element attribute:

var polylineElement = document.createElementNS("http://www.w3.org/2000/svg", "polyline");
var animatedPoints = polylineElement.points;
var points = animatedPoints.baseVal;
var point = points.appendItem(new DOMPoint(0, 0));
point.setX(100);
console.log(polylineElement.getAttribute("points")); // This should print 100, 0

So in this example:

1. The owner of 'point' is 'points' i.e the owner of DOMPoint is SVGPointList.
2. The owner of 'points' is 'animatedPoints' i.e the owner of SVGPointList is SVGAnimatedPointList
3. The owner of 'animatedPoints' is 'polylineElement' i.e the owner SVGAnimatedPointList is SVGPolylineElement.

To have this working this way, the following changes can be made to the C++ classes declarations:

class DOMPoint final : public DOMPointReadOnly {
private:
    DOMDataOwner* m_owner { nullptr };
};

class SVGPointList final : public DOMDataOwner {
{
    DOMDataOwner* owner() const override { return m_owner; }
    void commitChange() override;
private:
    DOMDataOwner* m_owner { nullptr };
};

class SVGAnimatedPointList : public DOMDataOwner {
    DOMDataOwner* owner() const override { return m_element; }
private:
    SVGElement* m_element { nullptr };
};

class SVGElement : public DOMDataOwner {
{
    void commitChange() override;
};

And here is simplified C++ code that can make a change in DOMPoint be propagated to SVGPointList, SVGAnimatedPointList and SVGPolylineElement without having to have a virtual function in DOMPoint:

void DOMPoint::setX(double x)
{ 
    m_x = x;
    if (m_owner)
        m_owner->commitChange();
}

void SVGPointList::commitChange()
{
    setDirty(true);
    DOMDataOwner::commitChange();
}

void SVGElement::commitChange()
{
    for (auto& property : m_properties) {
        if (property.isDirty())
            Element::setAttribute(attributeName(property), property.valueAsString());
    }
}

DOMPoint, SVGPointList SVGAnimatedPointList and SVGElement should manage attaching to and detaching from the DOMDataOwner.

1. When an SVGElement creates an SVG property, the SVG property is attached to the SVGElement.
2. When an SVGElement is being deleted, it is detached from all its SVG properties
3. When an SVGAnimatedProperty or an SVGAnimatedPropertyList is created, its baseVal and animVal are attached to the SVGAnimatedProperty or the SVGAnimatedPropertyList.
4. When an SVGAnimatedProperty or SVGAnimatedPropertyList is being deleted, it is detached from its baseVal and animVal.
5. The specs of SVGList.appendItem, SVGList.removeItem, SVGList.replaceItem and SVGList.initialize define well how they are attached to and detached from items.
6. When an SVGList is being deleted, it is detached from all its items

So if DOMPoint, SVGPointList or SVGAnimatedPointList is detached, i.e. its owner() is null, the commitChange() will stop since there is no owner to propagate the change to.

Note: I do not know whether exposing DOMDataOwner through the IDL is beneficial or not.

[shallawa]

I tested the latest builds of Chrome and FireFox and they have not implemented this change.

I think the livability issue is the problem. DOMPoint, DOMRect and DOMMatrix do not say anything about notifying "an observer" once their values are changed. This behavior is unique to SVG properties. The change in an SVG property must be re-serialized to the element attribute.

I tried to remove the SVGPoint and make DOMPoint a livable object. But I had to change the IDL a little and the C++ class does not match the IDL anymore. See the patch https://bugs.webkit.org/attachment.cgi?id=380154&action=review. I could not land this patch because the first question I was asked why Chrome and FireFox have not implemented it.

Can we get agreement on this topic or revert it please?

[progers]

@shallawa, I agree with @fsoder's description of the situation in chromium/blink: making this change is not entirely trivial, though is not hard, and this area has not been high on the priority list. This has resulted in no change.

Given the lack of active or even planned implementation work in this area, I think it makes sense to revert back to the SVGPoint types (@longsonr's proposal) to reflect the reality of implementations, though it is a step backwards in terms of long-term goals (html & svg interop, etc).

[AmeliaBR]

I think this counts as the feedback we were waiting for (in the July discussion), so adding back to the agenda for a final decision.

As for myself: most of the SVG DOM is legacy design. The long-term goals should be focused on integrating the Geometry interfaces with CSS Typed OM. Which might still require smarter "live" interfaces that reflect properties. But hopefully without the extra layers of misdirection that the SVGAnimatedXXX interfaces created.

[dirkschulze]

WebKit seems to be actively working on implementing the spec changes. I don't see a reasons to revert the changes right now especially since this is in the interest of web developers to harmonize the web world - a strong focus of SVG 2. I'd rather encourage other browser implementers to take another look at this point.

From the comments of @progers @longsonr it rather sounds like a priority and maybe complexity problem than a lack of implementability. Is there anything on the specification side that is unclear or should be changed to make the progress of aliasing SVGPoint and others easier?

[saschanaz]

But I had to change the IDL a little and the C++ class does not match the IDL anymore.

What specific mismatches were there, BTW? Could the IDL be fixed?

[shallawa]

But I had to change the IDL a little and the C++ class does not match the IDL anymore.

What specific mismatches were there, BTW? Could the IDL be fixed?

1. C++ DOMPoint has to inherit from DOMPointReadOnly and a new class which I named it DOMLiveProperty in WebKit. This makes DOMPoint owns a virtual table while DOMPointReadOnly does not. It also makes the sizeof(DOMPoint) != sizeof(DOMPointReadOnly). So casting DOMPointReadOnly to DOMPoint is not an option anymore.
2. DOMPoint can actually be read only. For example 'ployLineElement.animatedPoints[0].x = 10' has throw an exception. This is a little bit awkward since the DOMPoint is supposed to provide a modifiable point object but because its SVGElement decides it can't be modified, this DOMPoint behaves as if it were a DOMPointReadOnly.
3. The C++ DOMPoint needs to provide two sets of functions for setting its members. For example for setting the 'x' member we still need the function

void setX(double x) { m_x = x; }

This function is used only internally. But for the DOM binding we need something like this:

ExceptionOr<void> setXForBindings(double x)
{
    if (isReadOnly())
        return Exception { NoModificationAllowedError };

    m_x = x;
    commitChange();
    return { };
}

This function throws an exception if the SVGElement says it is readonly. Otherwise it tells the SVGElement to update the attribute.

Two problems I see with the current IDL:

1. DOMPoint.IDL has to mention somehow it can be a live object
2. There is a discrepancy between SVGPoint and DOMPoint/DOMPointReadOnly specification. The SVGPoint can be modifiable or read-only. It throws exception if it being modified and is readonly. But DOMPoint/DOMPointReadOnly tries to solve the readonly issue this by having two separate classes which can be casted to each other. In my patch I made casting DOMPointReadOnly to DOMPoint is not an option anymore and I made DOMPoint throws exception when it's "ReadOnly" which I think should not happen.

[smfr]

WebKit seems to be actively working on implementing the spec changes. I don't see a reasons to revert the changes right now especially since this is in the interest of web developers to harmonize the web world - a strong focus of SVG 2. I'd rather encourage other browser implementers to take another look at this point.

The outcome of WebKit's attempt to implement is a realization that DOMPoint etc. would have to be complicated in order to make things work. At this point, WebKit's opinion is not to change and to keep the SVG geometry types.

[css-meeting-bot]

The SVG Working Group just discussed Revert change from SVGRect/SVGPoint/SVGMatrix to DOM equivalents, and agreed to the following:

• RESOLVED: Revert decision to make SVG* geommetry interfaces aliases of the DOM* geometry interfaces (pending CSS WG approval)
• RESOLVED: Change the SVG spec to use the SVG* geometry interfaces instead of DOM* interfaces
• RESOLVED: Keep the usage of DOM*Init types in the SVG spec
• RESOLVED: Ask CSS WG to approve separating out most of the geometry interfaces into mixins we can reference in SVG interface definitions.

The full IRC log of that discussion

<AmeliaBR> Topic: Revert change from SVGRect/SVGPoint/SVGMatrix to DOM equivalents
<AmeliaBR> github: https://github.com/w3c/svgwg/issues/706
<AmeliaBR> Myles: We (Apple/WebKit) did some investigation. Agree with the OP (Robert Longson/Firefox) that we'd prefer to keep the SVG/complicated types separate from the simpler DOM types
<AmeliaBR> krit: And there's support in the issue from Chromium devs, too.
<AmeliaBR> ... The DOM Geometry spec was designed to harmonize the existing SVG types with a broader use case, so this would revert that.
<AmeliaBR> myles: But the reality is that we have, de facto, 2 different behaviors: live elements vs non-live. Encoding that difference formally in the type system makes it easier for authors to understand what's happening. Vs if we have one type that is sometimes live & sometimes not, it's harder to predict performance characteristics.
<AmeliaBR> Amelia: I agree that there is benefit of having clear type definitions. Especially re read-only types, it's currently very confusing.
<AmeliaBR> krit: For the read-only types, if we remove the alias, do we still have a need for the read-only versions?
<AmeliaBR> Amelia: I don't think it affects it. The SVG DOM doesn't use the read only DOM types, because read only constraints are two levels up in the nested objects.
<AmeliaBR> krit: Question is, what is the use case for the read-only types if they're not connected to live reflection.
<AmeliaBR> Amelia: I don't know. Performance, maybe?
<AmeliaBR> krit: I don't think there's an implementation impact.
<AmeliaBR> krit: In addition to the base types, we also now have all these *Init types. What to do about them?
<AmeliaBR> Amelia: They should be kept. & they will cover most of the harmonization, anyway. All parameters take the init types, so both the SVG* and DOM* versions will work, passing as parameter's to each others methods.
<AmeliaBR> Amelia: Other thing to consider. We've got methods & stuff that we don't want to spec twice. So we should probably extract things out to a mixin. E.g., a Point mixin that is implemented by both DOMPoint and SVGPoint.
<AmeliaBR> krit: That's reasonable. But I want to avoid too much change to the Geometry spec. Currently the change is mostly at the SVG level. SVG defines the aliases.
<AmeliaBR> Amelia: What status is Geometry Interfaces?
<AmeliaBR> krit: CR. But it's more than just process, it's also having to do the work.
<AmeliaBR> myles: I also think that mixin model is the way to go.
<AmeliaBR> krit: That would need approval from CSS WG, for changes to Geometry.
<AmeliaBR> ... We could agree to change the SVG spec to use the SVG* names in all the IDL. But currently, that wouldn't have an effect. They're defined as aliases.
<AmeliaBR> Amelia: SVG spec would need to define the SVG* interfaces, but they should use mixins defined in Geometry.
<AmeliaBR> krit: Proposed resolution #1: revert decision to make SVG* geommetry interfaces aliases of the DOM* geometry interfaces
<AmeliaBR> ... Proposed resolution 2: Change the SVG spec to use the SVG* interfaces instead of DOM*
<AmeliaBR> Amelia: Except for where we've already switched to using DOM*Init types
<AmeliaBR> krit: That should be a separate resolution.
<AmeliaBR> Amelia: Then 4th res would be: Ask CSS WG to approve separating out most of the geometry interfaces into mixins we can reference.
<AmeliaBR> krit: and to accept reversal of aliasing.
<AmeliaBR> Amelia: That's also defined in DOM Geometry?
<AmeliaBR> krit: yes.
<AmeliaBR> krit: I don't object to prop res 1 & 2, but I want to record that this is because of implementor priorities & could change in future. I'm also not voting in favour.
<AmeliaBR> Amelia: So you're abstaining?
<AmeliaBR> krit: yes.
<AmeliaBR> RESOLVED: Revert decision to make SVG* geommetry interfaces aliases of the DOM* geometry interfaces (pending CSS WG approval)
<AmeliaBR> RESOLVED: Change the SVG spec to use the SVG* geometry interfaces instead of DOM* interfaces
<AmeliaBR> RESOLVED: Keep the usage of DOM*Init types in the SVG spec
<AmeliaBR> RESOLVED: Ask CSS WG to approve separating out most of the geometry interfaces into mixins we can reference in SVG interface definitions.