Opened 12 years ago

Closed 4 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 12 years ago by Eugene Lazutkin

Status: newassigned
Summary: Bounding box calculations is broken for many shapes.Bounding box calculations are broken for many shapes.

comment:2 Changed 12 years ago by Eugene Lazutkin

(In [8737]) Partial fix for getTransformedBoundingBox(). Refs #3145.

comment:3 Changed 12 years ago by Eugene Lazutkin

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 12 years ago by Eugene Lazutkin

This feature is rarely used, and the bug fix doesn't change the API --- I am retargeting it to 0.9.

comment:5 Changed 12 years ago by Eugene Lazutkin

Milestone: 0.9beta0.9

comment:6 Changed 12 years ago by Eugene Lazutkin

Milestone: 0.91.0

comment:7 Changed 12 years ago by Eugene Lazutkin

Component: gfx (svg+vml)DojoX GFX

comment:8 Changed 12 years ago by Eugene Lazutkin

Type: defectenhancement

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 12 years ago by Eugene Lazutkin

Milestone: 1.02.0

comment:10 Changed 11 years ago by alex

Milestone: 2.01.3

Milestone 2.0 deleted

comment:11 Changed 11 years ago by Eugene Lazutkin

Milestone: 1.3future

Moving all open ticketd to the future.

comment:12 Changed 9 years ago by Eugene Lazutkin

(In [21698]) Better bounding box calculations for Path, !strict, refs #3145.

comment:13 Changed 9 years ago by Eugene Lazutkin

(In [21699]) Implementing an SVG-like linear gradient for VML renderer, better calculations of bounding box for Path, !strict, refs #3145, refs #10937.

comment:14 Changed 9 years ago by liyang

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 in reply to:  14 Changed 9 years ago by Eugene Lazutkin

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 9 years ago by liyang

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 Changed 9 years ago by 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?

comment:18 in reply to:  17 Changed 9 years ago by Eugene Lazutkin

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 9 years ago by Eugene Lazutkin

(In [21928]) Reverted the change to getTransformedBoundingBox, moved the new code to a private method, fixed a bug introduced with this change, !strict, refs #3145.

comment:20 Changed 4 years ago by dylan

Milestone: future1.12
Resolution: patchwelcome
Status: assignedclosed

Given the lack of activity over the past 6 years, I'm closing this as patchwelcome.

Note: See TracTickets for help on using tickets.