Opened 14 years ago
Closed 5 years ago
#3145 closed enhancement (patchwelcome)
Bounding box calculations are broken for many shapes.
Reported by: | Eugene Lazutkin | Owned by: | Eugene Lazutkin |
---|---|---|---|
Priority: | high | Milestone: | 1.13 |
Component: | DojoX GFX | Version: | 0.9 |
Keywords: | Cc: | liyang | |
Blocked By: | Blocking: |
Description
Change History (20)
comment:1 Changed 14 years ago by
Status: | new → assigned |
---|---|
Summary: | Bounding box calculations is broken for many shapes. → Bounding box calculations are broken for many shapes. |
comment:2 Changed 14 years ago by
comment:3 Changed 14 years ago by
Now I need to add invalidation of bbox, when shape is changed or when shapes are manipulated within a group (added/deleted/transformed).
comment:4 Changed 14 years ago by
This feature is rarely used, and the bug fix doesn't change the API --- I am retargeting it to 0.9.
comment:5 Changed 14 years ago by
Milestone: | 0.9beta → 0.9 |
---|
comment:6 Changed 14 years ago by
Milestone: | 0.9 → 1.0 |
---|
comment:7 Changed 14 years ago by
Component: | gfx (svg+vml) → DojoX GFX |
---|
comment:8 Changed 13 years ago by
Type: | defect → enhancement |
---|
This is rarely needed functionality. It should be split off in the same fashion as attach() functionality is split off, so we can reduce the size of renderers.
comment:9 Changed 13 years ago by
Milestone: | 1.0 → 2.0 |
---|
comment:12 Changed 11 years ago by
comment:13 Changed 11 years ago by
comment:14 follow-up: 15 Changed 11 years ago by
I have concerns in the change introduced in [21698]. The bounding box it implements is a "different" one rather than a better one. What [21698] implements is "an axis-aligned-bbox of transformed shape", while the previous impl was "a transformed shape bbox". They are very different in concept.
My understanding is the contract of getTransformedBoundingBox() is to return "a transformed shape bbox". And that's how it was implemented across all gfx shapes. If for some reason we decided to change the contract, then it must be carried out consistently to other shapes as well.
Btw, how can I add myself to the CC of this ticket? I want to keep informed on this one.
comment:15 Changed 11 years ago by
Cc: | liyang added |
---|
Replying to liyang:
My understanding is the contract of getTransformedBoundingBox() is to return "a transformed shape bbox". And that's how it was implemented across all gfx shapes. If for some reason we decided to change the contract, then it must be carried out consistently to other shapes as well.
"Transformed bbox" is used mostly because of its simplicity, not because it is correct, or extremely useful. In general we need to use native methods to calculate both bounding boxes, yet VML (and Canvas? and Silverlight 1.0? and earlier versions of SVG as implemented by browsers) do not provide this functionality => that's why we have what we have. I think it is time to revise this approach.
That is why we have this ticket and it is still open.
comment:16 Changed 11 years ago by
Thanks for adding me to CC and explaining the background.
However I still see the need of the old "Transformed bbox", e.g. when resizing a rotated object in a drawing tool, and for those who're relying on the current contract of the method, they will be disappointed if we simply break the current impl. Maybe adding a new API for the new contract is a better option?
comment:17 follow-up: 18 Changed 11 years ago by
The change made in [21698] introduced some defect and breaks test cases test_fx, test_tbbox, and test_transform on trunk. Also blocks some of my work in parallel.
Eugene, can we have a stable version on trunk? Either rollback to previous or fix the current defect?
comment:18 Changed 11 years ago by
Replying to liyang:
The change made in [21698] introduced some defect and breaks test cases test_fx, test_tbbox, and test_transform on trunk. Also blocks some of my work in parallel.
Eugene, can we have a stable version on trunk? Either rollback to previous or fix the current defect?
After thinking it over I think that you are right. I'll revert the change, and move the current implementation of getTransformedBoundingBox()
to a private method.
comment:19 Changed 11 years ago by
comment:20 Changed 5 years ago by
Milestone: | future → 1.12 |
---|---|
Resolution: | → patchwelcome |
Status: | assigned → closed |
Given the lack of activity over the past 6 years, I'm closing this as patchwelcome.
(In [8737]) Partial fix for getTransformedBoundingBox(). Refs #3145.