Re: [webkit-dev] Int/FloatPoint and Int/FloatSize
On Jan 9, 2013, at 5:40 PM, Simon Fraser wrote: > On Jan 9, 2013, at 4:52 PM, Steve Block wrote: > >>> I'm really not sure that this set of changes is going in the right >>> direction. What's driving them; some abstract sense of purity, or >>> reducing the chances of introducing real bugs that we've seen in the past? >> Some of both. I was investigating a bug where WebCore is generating >> unexpected negative sizes. While looking into how this could happen, I >> noticed that in some cases, negative sizes result from size types >> being used to represent things that seem more like points. It seems >> like it would be good to clean this up this misuse of size types, and >> perhaps in the long term, we can avoid negative sizes altogether, >> which would help avoid these kinds of bug in the future. > > It seems like a lot of churn for relatively little gain. I'd rather our time > be focused > on areas that actually benefit users of WebKit. I agree with this concern. Another object for representing 2D vector data will cause more maintenance cost now and in the future. And I doubt that it gives a big enough benefit over defining when to use Point and Size today. Greetings, Dirk > > Simon > > > ___ > webkit-dev mailing list > webkit-dev@lists.webkit.org > http://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Int/FloatPoint and Int/FloatSize
> It seems like a lot of churn for relatively little gain. I'd rather our time > be focused > on areas that actually benefit users of WebKit. OK. I don't feel strongly enough about this to push the issue. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Int/FloatPoint and Int/FloatSize
On Jan 9, 2013, at 4:52 PM, Steve Block wrote: >> I'm really not sure that this set of changes is going in the right >> direction. What's driving them; some abstract sense of purity, or >> reducing the chances of introducing real bugs that we've seen in the past? > Some of both. I was investigating a bug where WebCore is generating > unexpected negative sizes. While looking into how this could happen, I > noticed that in some cases, negative sizes result from size types > being used to represent things that seem more like points. It seems > like it would be good to clean this up this misuse of size types, and > perhaps in the long term, we can avoid negative sizes altogether, > which would help avoid these kinds of bug in the future. It seems like a lot of churn for relatively little gain. I'd rather our time be focused on areas that actually benefit users of WebKit. Simon ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Int/FloatPoint and Int/FloatSize
> I'm really not sure that this set of changes is going in the right direction. > What's driving them; some abstract sense of purity, or > reducing the chances of introducing real bugs that we've seen in the past? Some of both. I was investigating a bug where WebCore is generating unexpected negative sizes. While looking into how this could happen, I noticed that in some cases, negative sizes result from size types being used to represent things that seem more like points. It seems like it would be good to clean this up this misuse of size types, and perhaps in the long term, we can avoid negative sizes altogether, which would help avoid these kinds of bug in the future. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Int/FloatPoint and Int/FloatSize
On Wed, Jan 9, 2013 at 3:47 PM, Steve Block wrote: > > Also, the word "position" is used to represent a tree-position in DOM in > > WebKit so please don't use that to represent a point in a screen or a > layout > > coordinate system. > OK, perhaps we should use Vector2d, as is used by the Chromium port > My reference, if we’re doing this rename after addressing concerns expressed by Simon and others, is TwoDimensionalVector, R2Vector, or VectorInR2 because it’s a vector in a two-dimensional vector space, not a "2d" of type vector. - R. Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Int/FloatPoint and Int/FloatSize
> Removing the existing subtraction operator (xxxPoint minus > xxxPoint returns xxxSize) might be a good place to start. I've uploaded a patch to https://bugs.webkit.org/show_bug.cgi?id=106408 which replaces these subtraction operators with ones that return xxxPoint, and which adds Int/FloatSize::fromCornerPoints(). The patch does just enough to make these changes, though in many cases, the switch from xxxSize to xxxPoint could be propagated further up and down the stack. However, the patch is probably big enough as it is, so this should probably be done separately, on a case-by-case basis. Note that the patch isn't ready for formal review, but I wanted to get some feedback before I pursue it further. Steve ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Int/FloatPoint and Int/FloatSize
On Jan 9, 2013, at 3:47 PM, Steve Block wrote: >> Also, the word "position" is used to represent a tree-position in DOM in >> WebKit so please don't use that to represent a point in a screen or a layout >> coordinate system. > OK, perhaps we should use Vector2d, as is used by the Chromium port I don't like that name. I'm really not sure that this set of changes is going in the right direction. What's driving them; some abstract sense of purity, or reducing the chances of introducing real bugs that we've seen in the past? Simon ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Int/FloatPoint and Int/FloatSize
> Also, the word "position" is used to represent a tree-position in DOM in > WebKit so please don't use that to represent a point in a screen or a layout > coordinate system. OK, perhaps we should use Vector2d, as is used by the Chromium port ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Int/FloatPoint and Int/FloatSize
On Fri, Jan 4, 2013 at 12:15 AM, Simon Fraser wrote: > On Jan 3, 2013, at 7:43 PM, Steve Block wrote: > > > Thanks all for the detailed replies. > > > > I wasn't aware of the distinction made between points and vectors for > > the purposes transforms. However, if I understand things correctly, > > introducing a vector type could be considered separately from the > > issue I initially raised. > > > > It seems that everyone is agreed that xxxSize should be used only when > > you really want to represent a size. A good first step would be trying > > to enforce this, such that all points and vectors are represented with > > xxxPoint. As Shawn points out, when doing this, we need to make sure > > that we continue to use the correct homogeneous coordinate for > > transforms. Removing the existing subtraction operator (xxxPoint minus > > xxxPoint returns xxxSize) might be a good place to start. > > I find point - point = size quite useful in general, and it seems to make > logical sense. > > > > > Introducing the concept of a vector could then be done in a second phase. > > What would you call this type, avoiding confusion with Vector<>? > In chromium we recently added such a class with the name Vector2d, to differentiate it from std::vector. > > Simon > > ___ > webkit-dev mailing list > webkit-dev@lists.webkit.org > http://lists.webkit.org/mailman/listinfo/webkit-dev > ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Int/FloatPoint and Int/FloatSize
Subtraction of two points should not equate to a size. That's would be unexpected behavior for anyone used to working in 2D space. Mathematically speaking point - point equates to a vector. When writing a vector math library there typically isn't a differentiation made between a point and a vector, mainly because there's a lot of code overlap between the two. However when they do for 3D values a point is x, y, z, 1 while a vector is x, y, z, 0. On a somewhat unrelated note is there any interest in getting a SIMD implementation of these classes? From: webkit-dev-boun...@lists.webkit.org [mailto:webkit-dev-boun...@lists.webkit.org] On Behalf Of Ryosuke Niwa Sent: Friday, January 04, 2013 12:19 AM To: Steve Block Cc: WebKit-Dev Development Subject: Re: [webkit-dev] Int/FloatPoint and Int/FloatSize On Thu, Jan 3, 2013 at 10:32 PM, Steve Block mailto:stevebl...@chromium.org>> wrote: > I find point - point = size quite useful in general, and it seems to make > logical sense. I agree that it makes logical sense, but I think that 'point - point = point' also makes sense, and is perhaps more frequently the right choice. > What would you call this type, avoiding confusion with Vector<>? I guess 'Offset' is an obvious candidate, but that is probably already too overloaded. Perhaps RelativePosition or RelativePoint? I don't think "Relative" adds any value here. Also, the word "position" is used to represent a tree-position in DOM in WebKit so please don't use that to represent a point in a screen or a layout coordinate system. I don't like "offset" either because it's such an overloaded word. e.g. "offset" is often used to mean a child node index in DOM. - R. Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Int/FloatPoint and Int/FloatSize
On Thu, Jan 3, 2013 at 10:32 PM, Steve Block wrote: > > I find point - point = size quite useful in general, and it seems to > make logical sense. > I agree that it makes logical sense, but I think that 'point - point = > point' also makes sense, and is perhaps more frequently the right > choice. > > > What would you call this type, avoiding confusion with Vector<>? > I guess 'Offset' is an obvious candidate, but that is probably already > too overloaded. Perhaps RelativePosition or RelativePoint? > I don't think "Relative" adds any value here. Also, the word "position" is used to represent a tree-position in DOM in WebKit so please don't use that to represent a point in a screen or a layout coordinate system. I don't like "offset" either because it's such an overloaded word. e.g. "offset" is often used to mean a child node index in DOM. - R. Niwa ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Int/FloatPoint and Int/FloatSize
> I find point - point = size quite useful in general, and it seems to make > logical sense. I agree that it makes logical sense, but I think that 'point - point = point' also makes sense, and is perhaps more frequently the right choice. > What would you call this type, avoiding confusion with Vector<>? I guess 'Offset' is an obvious candidate, but that is probably already too overloaded. Perhaps RelativePosition or RelativePoint? On 4 January 2013 16:15, Simon Fraser wrote: > On Jan 3, 2013, at 7:43 PM, Steve Block wrote: > >> Thanks all for the detailed replies. >> >> I wasn't aware of the distinction made between points and vectors for >> the purposes transforms. However, if I understand things correctly, >> introducing a vector type could be considered separately from the >> issue I initially raised. >> >> It seems that everyone is agreed that xxxSize should be used only when >> you really want to represent a size. A good first step would be trying >> to enforce this, such that all points and vectors are represented with >> xxxPoint. As Shawn points out, when doing this, we need to make sure >> that we continue to use the correct homogeneous coordinate for >> transforms. Removing the existing subtraction operator (xxxPoint minus >> xxxPoint returns xxxSize) might be a good place to start. > > I find point - point = size quite useful in general, and it seems to make > logical sense. > >> >> Introducing the concept of a vector could then be done in a second phase. > > What would you call this type, avoiding confusion with Vector<>? > > Simon > ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Int/FloatPoint and Int/FloatSize
On 4 January 2013 16:15, Simon Fraser wrote: > > Introducing the concept of a vector could then be done in a second phase. > > What would you call this type, avoiding confusion with Vector<>? > > Rename that to DynamicallyResizableRandomlyAccessibleT<>, because, you know, Euclid got there first :P ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Int/FloatPoint and Int/FloatSize
On Jan 3, 2013, at 7:43 PM, Steve Block wrote: > Thanks all for the detailed replies. > > I wasn't aware of the distinction made between points and vectors for > the purposes transforms. However, if I understand things correctly, > introducing a vector type could be considered separately from the > issue I initially raised. > > It seems that everyone is agreed that xxxSize should be used only when > you really want to represent a size. A good first step would be trying > to enforce this, such that all points and vectors are represented with > xxxPoint. As Shawn points out, when doing this, we need to make sure > that we continue to use the correct homogeneous coordinate for > transforms. Removing the existing subtraction operator (xxxPoint minus > xxxPoint returns xxxSize) might be a good place to start. I find point - point = size quite useful in general, and it seems to make logical sense. > > Introducing the concept of a vector could then be done in a second phase. What would you call this type, avoiding confusion with Vector<>? Simon ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Int/FloatPoint and Int/FloatSize
Thanks all for the detailed replies. I wasn't aware of the distinction made between points and vectors for the purposes transforms. However, if I understand things correctly, introducing a vector type could be considered separately from the issue I initially raised. It seems that everyone is agreed that xxxSize should be used only when you really want to represent a size. A good first step would be trying to enforce this, such that all points and vectors are represented with xxxPoint. As Shawn points out, when doing this, we need to make sure that we continue to use the correct homogeneous coordinate for transforms. Removing the existing subtraction operator (xxxPoint minus xxxPoint returns xxxSize) might be a good place to start. Introducing the concept of a vector could then be done in a second phase. WDYT? Steve ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Int/FloatPoint and Int/FloatSize
On Thu, Jan 3, 2013 at 3:14 PM, Peter Kasting wrote: > On Thu, Jan 3, 2013 at 11:36 AM, Shawn Singh wrote: > >> Cons of making a separate vector class: >> - "offsets" are sometimes treated as relative point locations, and >> other times treated as vectors that can be added to points. Deciding when >> to use which one could become just as confusing as using Point vs Size is >> right now. >> > > Yeah, this is a real danger. It's sort of mitigated if you have no way to > add/subtract two points, only a point and a vector, because that kind of > forces you to always use your vector class for offsets, otherwise you can't > do much with them. However, you do still end up needing things like > "PointAtOffsetFromOrigin(const vector&)" that basically just convert a > vector directly to a point, so there is still the possibility for confusion. > > PK > The homogenous coordinate issue is the primary reason why it matters whether you are using (x,y) to represent a point or a direction. We seem to be begging for bugs by failing to clearly distinguish between the two cases. Not everyone learns this stuff, so here the summary. Say you have a transformation T(..) that transforms points, and it's using homogeneous coordinates. It is desirable to have T(a-b) == T(a) - T(b). To do so requires storing a-b, which is really a direction or offset, with a zero homogeneous coordinate. I would support adding "offset" and enforcing the mathematical definitions through parameter and return types, which is basically what Peter is suggesting. I think it makes the code much more self-documenting and will reduce the chance of bugs. Cheers, Stephen.Stephen Chenney | Software Engineer | schen...@google.com | 404-314-1809 ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Int/FloatPoint and Int/FloatSize
On Thu, Jan 3, 2013 at 11:36 AM, Shawn Singh wrote: > Cons of making a separate vector class: > - "offsets" are sometimes treated as relative point locations, and other > times treated as vectors that can be added to points. Deciding when to use > which one could become just as confusing as using Point vs Size is right > now. > Yeah, this is a real danger. It's sort of mitigated if you have no way to add/subtract two points, only a point and a vector, because that kind of forces you to always use your vector class for offsets, otherwise you can't do much with them. However, you do still end up needing things like "PointAtOffsetFromOrigin(const vector&)" that basically just convert a vector directly to a point, so there is still the possibility for confusion. PK ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Int/FloatPoint and Int/FloatSize
Personally I like the idea of removing the subtraction operator (point minus point returns size) and make it explicit. *** However ***, if we change the data type of objects from Size to Point, we have to be careful to check whether they are ever mapped by transforms. In particular, Points use a homogeneous coordinate of w=1, while Size (and conceptual vectors) use homogeneous coordinates of w=0. Some more thinking out loud - Pros of making a separate vector class: - avoids the mismatch in Size or Point APIs - avoids the mismatch in homogeneous coordinate w usage - would allow us more compile-time safety in usage of math objects, i.e. doing an operation that doesn't make sense could cause a type-mismatch error. Cons of making a separate vector class: - "offsets" are sometimes treated as relative point locations, and other times treated as vectors that can be added to points. Deciding when to use which one could become just as confusing as using Point vs Size is right now. On Thu, Jan 3, 2013 at 10:00 AM, Levi Weintraub wrote: > Hi Steve, > > When converting the old tx/ty paint offsets to use IntPoint and IntSize > (later LayoutPoint/Size) we had some discussion around this. Darin Adler > wrote some good advice here: > https://bugs.webkit.org/show_bug.cgi?id=61562#c2 -- quoting: > "It’s hard to tell points and sizes apart when we have nested coordinate > systems. The distance from the top left to a rect is a “size”, yet it’s > expressed as an origin point. I think that whenever we can’t decide, we > should use IntPoint, and we should use IntSize only when it’s clearly the > size of something, not just a distance (a point described relative to > another point)." > > Cheers, > -Levi > > > On Wed, Jan 2, 2013 at 11:21 PM, Steve Block wrote: > >> Hi webkit, >> >> I was hoping that somebody could clarify the policy regarding the >> correct use of Int/FloatPoint vs Int/FloatSize. >> >> It seems that xxxPoint is consistently used to represent a position, >> which makes sense. However, when representing the position of one >> point relative to another, both xxxPoint and xxxSize are used, which >> seems inconsistent. I'd expect that xxxPoint should be used for this >> case of a relative position, since its x(), y() and length() methods >> make more sense than the width() and height() methods of xxxSize. >> However, the operators [1] for subtracting one xxxPoint from another >> encourage the use of xxxSize. I recognize that in some situations, you >> need really do want to represent the difference between two points as >> an area or size, but this seems the less common case, and it might be >> better to make it more explicit [2]. >> >> My questions are ... >> - What (if any) is the correct policy? >> - Would people welcome changes to encourage that policy? >> >> Thanks, >> Steve >> >> [1] eg 'inline FloatSize operator-(const FloatPoint&, const FloatPoint&)' >> [2] Perhaps something like 'static FloatSize >> FloatSize::fromCornerPoints(const FloatPoint&, const FloatPoint&)'. >> ___ >> webkit-dev mailing list >> webkit-dev@lists.webkit.org >> http://lists.webkit.org/mailman/listinfo/webkit-dev >> > > > ___ > webkit-dev mailing list > webkit-dev@lists.webkit.org > http://lists.webkit.org/mailman/listinfo/webkit-dev > > ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Int/FloatPoint and Int/FloatSize
Hi Steve, When converting the old tx/ty paint offsets to use IntPoint and IntSize (later LayoutPoint/Size) we had some discussion around this. Darin Adler wrote some good advice here: https://bugs.webkit.org/show_bug.cgi?id=61562#c2 -- quoting: "It’s hard to tell points and sizes apart when we have nested coordinate systems. The distance from the top left to a rect is a “size”, yet it’s expressed as an origin point. I think that whenever we can’t decide, we should use IntPoint, and we should use IntSize only when it’s clearly the size of something, not just a distance (a point described relative to another point)." Cheers, -Levi On Wed, Jan 2, 2013 at 11:21 PM, Steve Block wrote: > Hi webkit, > > I was hoping that somebody could clarify the policy regarding the > correct use of Int/FloatPoint vs Int/FloatSize. > > It seems that xxxPoint is consistently used to represent a position, > which makes sense. However, when representing the position of one > point relative to another, both xxxPoint and xxxSize are used, which > seems inconsistent. I'd expect that xxxPoint should be used for this > case of a relative position, since its x(), y() and length() methods > make more sense than the width() and height() methods of xxxSize. > However, the operators [1] for subtracting one xxxPoint from another > encourage the use of xxxSize. I recognize that in some situations, you > need really do want to represent the difference between two points as > an area or size, but this seems the less common case, and it might be > better to make it more explicit [2]. > > My questions are ... > - What (if any) is the correct policy? > - Would people welcome changes to encourage that policy? > > Thanks, > Steve > > [1] eg 'inline FloatSize operator-(const FloatPoint&, const FloatPoint&)' > [2] Perhaps something like 'static FloatSize > FloatSize::fromCornerPoints(const FloatPoint&, const FloatPoint&)'. > ___ > webkit-dev mailing list > webkit-dev@lists.webkit.org > http://lists.webkit.org/mailman/listinfo/webkit-dev > ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Int/FloatPoint and Int/FloatSize
On Wed, Jan 2, 2013 at 11:21 PM, Steve Block wrote: > - Would people welcome changes to encourage that policy? FWIW, in Chromium (downstream, non-WebKit) we ended up adding a "vector" (as in math, not the STL) class recently to address the sort of "offset/delta between two points" use you're describing, because neither points nor sizes made for a good fit all the time. The folks working on Chromium's compositor might have more feedback, but personally I thought that was a good move, and it turned out to not be too hard to convert almost everything at once, simply by removing all the functions implementing mathematical operations between type pairs we no longer wanted to support and seeing what broke. Of course, such a drastic move in WebKit would need to have significant buy-in ahead of time, and my suspicion is that it wouldn't get it. PK ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
