First step needs to happen completely before the second step does, so I
guess the danger is that we start and give up before we get to step 2.
I don't think that will happen, but it is something we should always
think about.
Third step - sure, I can see this not getting completed - examining
places that do xmax = xmin + width instead of just getting the max X,
for example.
On 09-Feb-17 20:56, Nicholas Nethercote wrote:
My concern is that this will get partially done but never completed, and
we'll end up in a situation where we have two styles in place. We have a
bad track record with that sort of thing.
Nick
On Thu, Feb 9, 2017 at 1:45 PM, Jeff Muizelaar <jrmui...@gmail.com> wrote:
It's not very easy to reason about overflow issues with our current
representation. This means that we currently just pretend that they
don't happen. The idea for changing the representation came up in
response to a security bug where we didn't really have a better
solution.
Changing to x1, x2, y1, y2 will allow us to match the pixman rectangle
representation that we use for regions. The region code shows quite a
bit profiles so even a small improvement in this area is nice to have.
-Jeff
On Wed, Feb 8, 2017 at 9:19 PM, David Major <dma...@mozilla.com> wrote:
Is there a specific problem that's being solved by this proposal? It
would
be helpful to make this a bit more concrete, like "these benchmarks go x%
faster", or "here's a list of overflow bugs that will just vanish", or
"here's some upcoming work that this would facilitate".
On Thu, Feb 9, 2017 at 1:56 PM, Botond Ballo <bba...@mozilla.com> wrote:
Hi everyone!
I would like to propose changing the internal representation of
rectangles in platform code.
We currently represent a rectangle by storing the coordinates of its
top-left corner, its width, and its height. I'll refer to this
representation as "x/y/w/h".
I would like to propose storing instead the coordinates of the
top-left corner, and the coordinates of the bottom-right corner. I'll
refer to this representation as "x1/y1/x2/y2".
The x1/y1/x2/y2 representation has several advantages over x/y/w/h:
- Several operations are more efficient with x1/y1/x2/y2, including
intersection,
union, and point-in-rect.
- The representation is more symmetric, since it stores two quantities
of the
same kind (two points) rather than a point and a dimension
(width/height).
- The representation is less susceptible to overflow. With x/y/w/h,
computation
of x2/y2 can overflow for a large range of values of x/y and w/h.
However,
with x1/y1/x2/y2, computation of w/h cannot overflow if the
coordinates are
signed and the resulting w/h is unsigned.
A known disadvantage of x1/y1/x2/y2 is that translating the rectangle
requires translating both points, whereas translating x/y/w/h only
requires translating one point. I think this disadvantage is minor in
comparison to the above advantages.
The proposed change would affect the class mozilla::gfx::BaseRect, and
classes that derive from it (such as CSSRect, LayoutRect, etc., and,
notably, nsRect and nsIntRect), but NOT other rectangle classes like
DOMRect.
I would like to propose making the transition as follows:
- Replace direct accesses to the 'width' and 'height' fields
throughout
the codebase with calls to getter and setter methods. (There aren't
that many - on the order of a few dozen, last I checked.)
- Make the representation change, which is non-breaking now that
the direct accesses to 'width' and 'height' have been removed.
- Examine remaining calls to the getters and setters for width and
height and see if any can be better expressed using operations
on the points instead.
The Graphics team, which owns this code, is supportive of this change.
However, since this is a fundamental utility type that's used by a
variety of platform code, I would like to ask the wider platform
development community for feedback before proceeding. Please let me
know if you have any!
Thanks,
Botond
[1]
http://searchfox.org/mozilla-central/rev/672c83ed65da286b68be1d02799c35
fdd14d0134/gfx/2d/BaseRect.h#46
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
--
- Milan (mi...@mozilla.com)
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
