benloh
commented
5 years ago In GitLab by @daveseah on Oct 31, 2019, 10:29
Nice link! Having recently worked on fixing the jumpy behavior when moving props, want to answer this while it's fresh in my mind.
Q. Is there a better way to do it?
A. Excerpted the actual code, I think you're talking about this code?
var svg = document.getElementById('mysvg'),
local = svg.getElementById('local');
function svgPoint(element, x, y) {
var pt = svg.createSVGPoint();
pt.x = x;
pt.y = y;
return pt.matrixTransform(element.getScreenCTM().inverse());
}This is the correct general idea, though you wouldn't want to calculate the inverse of the transformation matrix on every call.
Also, a call like MatrixTransform tends to be pretty expensive (or used to be) in computer graphics, but it is perhaps unavoidable. With our small datasets and prototype-quality bar, we can afford inefficiency here until it becomes a reported problem. That said, I tend to at least structure the code so it's easier to inject performance enhancements later (pre-computing static values whenever possible, in this case, using table lookups, etc).
Q. Should we generalize this into a utility in default.jsx?
A. I think it should probably live in an SVG drawing module, but we don't have architecture for that because we didn't have the UI architecture in React yet when I made it. Maybe PMCView should be the home of it (we have an unfortunately-named SVGView JSX component too that gums up the hierarchy)
Q. We need access to the root svg element [...] best way?
A. Providing a getter isn't much better than grabbing the Id from the element. In the case of needing the SVG root element, I'd probably structure it by "functional intent" rather than by "raw data structure". The correct intent is to get a transformed point, not get a raw SVG object so I can then do the work myself. The OWNER of the raw SVG object should therefore encapsulate related functionality; an approach like const {x,y} = PMCView.TransformPointToScreen(x,y) makes more sense to me than providing a getter for the raw SVG object.
The drag and drop handler on vprops seems to prevent any events from bubbling down to any child components. For example, StickyNoteButtons (and VBadges) on VProps do not receive click events.
The solution seems to be to process the drag event in
class-vprop-dragdrop.jsduring the end of drag handler. There we can determine if the click happened inside the StickyNoteButton or if it should just be handled as a prop drag.The problem is that the drag event's mouse location is in screen coordinates. So if we check the click point is inside the StickyNoteButton, the check fails because the coordinate systems do not match.
This is exacerbated by the introduction of panzoom, which can significantly change the SVG coordinate system relative to screen coordinates.
I believe the solution is to translate the screen-based mouse clicks into SVG coordinates.
The following commit tries to do that:
798e0915a2fe66f427c03ddab98ce802795e6956
It works based on my testing. You can pan and zoom all you want and the sticky note buttons remain clickable.
Needing Review:
Is the technique from https://www.sitepoint.com/how-to-translate-from-dom-to-svg-coordinates-and-back-again/ the right technique? If not, is there a better way to do it?
Should we generalize this into a utility in
default.jsx?In order to calculate the svg coordinates, we need access to the root svg element, so I added an id and are retrieving it via
document.getElementById. This seems a little antiquated. Should there be another way to access the root svg? Perhaps via a pmc-view call? e.g.PMCView.GetSVGRoot() => return m_svgroot;