Re: [Development] matrix math help needed - https://bugreports.qt.io/browse/QTBUG-84441
Il 28/05/20 16:18, Matthew Woehlke ha scritto: While that may be true, changing it now is going to break*every* user that uses these methods to generate compound transformations... and it'll be a silent break. I would be*very* surprised if that doesn't generate more bug reports. I 100% agree, no behavioral changes (clarifications in the docs are welcome) can be made at this point. Thanks, -- Giuseppe D'Angelo | giuseppe.dang...@kdab.com | Senior Software Engineer KDAB (France) S.A.S., a KDAB Group company Tel. France +33 (0)4 90 84 08 53, http://www.kdab.com KDAB - The Qt, C++ and OpenGL Experts smime.p7s Description: Firma crittografica S/MIME ___ Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
Re: [Development] matrix math help needed - https://bugreports.qt.io/browse/QTBUG-84441
On 27/05/2020 11.09, Giuseppe D'Angelo via Development wrote: Sure, augmenting the docs would help. But the whole point of the API is for its usage to be straightforward. If you do QTransform t; t.translate(); t.rotate(); t.scale(); auto result = t.map(foo); the "obvious" meaning should be that foo is getting first translated, then rotated, then scaled; not the other way around. While that may be true, changing it now is going to break *every* user that uses these methods to generate compound transformations... and it'll be a silent break. I would be *very* surprised if that doesn't generate more bug reports. I think the *absolute best* we could do would be to add an optional parameter specifying in what order to apply the change, defaulted to the old value, with a macro to instead default it to the new value. Maybe we can then, eventually, make defaulting to the old behavior deprecated (either opt-in to the new, or explicitly specify), and eventually remove it and make the new behavior default. But we're probably talking 2-3 release cycles. Is it really worth it? -- Matthew ___ Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
Re: [Development] matrix math help needed - https://bugreports.qt.io/browse/QTBUG-84441
On 5/27/20 3:58 PM, Matthew Woehlke wrote: *Nothing* there clearly states, at least to my reading, whether the "new" transform happens*before* or*after* any existing transforms that the QTransform is already doing. IMO, changing this to clarify that would help significantly. Sure, augmenting the docs would help. But the whole point of the API is for its usage to be straightforward. If you do QTransform t; t.translate(); t.rotate(); t.scale(); auto result = t.map(foo); the "obvious" meaning should be that foo is getting first translated, then rotated, then scaled; not the other way around. If this is achieved by pre or postmultiplication of (transposed) matrices matters only if you're into Algebra™ -- i.e. poking into the actual matrix, or if you're combining two transforms by means of operator*. Otherwise, it is not interesting at all in 99% of the cases, where you'd just set the transform on a painter or an item similar. If you really want to use the "low levels", please also note that operator* is helping you: QTransform t1, t2; QTransform t = t1 * t2; // ok... auto result = foo * t; // can only premultiply! // operator*(t, foo) does not exist So you've built foo * t1 * t2, with t1 applied first. (This in turn should reveal how QTransform works internally.) My 2 c, -- Giuseppe D'Angelo | giuseppe.dang...@kdab.com | Senior Software Engineer KDAB (France) S.A.S., a KDAB Group company Tel. France +33 (0)4 90 84 08 53, http://www.kdab.com KDAB - The Qt, C++ and OpenGL Experts smime.p7s Description: S/MIME Cryptographic Signature ___ Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
Re: [Development] matrix math help needed - https://bugreports.qt.io/browse/QTBUG-84441
Hi, I think the documentation is actually clear on the order by the scale() example. I'm not sure whether we should at explicitly the concept of intrinsic transform: https://en.wikipedia.org/wiki/Euler_angles#Definition_by_intrinsic_rotations The current QTransform documentation always states transforming the coordinate system. Probably, it's supposed to be easier to understand compared the word "intrinsic". The order of extrinsic transforms would be exactly the opposite of intrinsic transforms. Regards, Dongxu On Wed, May 27, 2020 at 10:09 AM Edward Welbourne wrote: > > > Here is, for example, the documentation of QTransform::scale: > > > > Scales the coordinate system by sx horizontally and sy vertically, > > and returns a reference to the matrix. > > > > *Nothing* there clearly states, at least to my reading, whether the > > "new" transform happens *before* or *after* any existing transforms that > > the QTransform is already doing. > > ___ > Development mailing list > Development@qt-project.org > https://lists.qt-project.org/listinfo/development > -- Dongxu Li, Ph.D. ___ Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
Re: [Development] matrix math help needed - https://bugreports.qt.io/browse/QTBUG-84441
Matthew Woehlke (26 May 2020 18:15) wrote: >>> The documentation is not clear if the scale, rotate, etc. methods of >>> QTransform apply *before* or *after* whatever the QTransform is already >>> doing. The bug report indicates that they are applied *first*. >>> >>> Given the potential for breaking existing code which expects the current >>> behavior, my inclination would be to clarify the documentation to >>> clearly state the existing behavior. On 27/05/2020 04.34, Edward Welbourne wrote: >> Yes, the docs do need updated; they do correctly say what QTransform does Matthew Woehlke (27 May 2020 15:58) > Really? Where? In the example code it includes. Not that I'm saying this is a good way to convey what's happening, but it did tell me everything I needed to know to work out what QTransform does. > Here is, for example, the documentation of QTransform::scale: > > Scales the coordinate system by sx horizontally and sy vertically, > and returns a reference to the matrix. > > *Nothing* there clearly states, at least to my reading, whether the > "new" transform happens *before* or *after* any existing transforms that > the QTransform is already doing. Indeed, although the class comment does say things from which it can be worked out - though I'm not sure every reader can be expected to. > IMO, changing this to clarify that would help significantly. No disagreement here, as I said (and you quoted): >> Yes, the docs do need updated; Eddy. ___ Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
Re: [Development] matrix math help needed - https://bugreports.qt.io/browse/QTBUG-84441
On 27/05/2020 04.34, Edward Welbourne wrote: Matthew Woehlke (26 May 2020 18:15) wrote: The documentation is not clear if the scale, rotate, etc. methods of QTransform apply *before* or *after* whatever the QTransform is already doing. The bug report indicates that they are applied *first*. Given the potential for breaking existing code which expects the current behavior, my inclination would be to clarify the documentation to clearly state the existing behavior. Yes, the docs do need updated; they do correctly say what QTransform does Really? Where? Here is, for example, the documentation of QTransform::scale: Scales the coordinate system by sx horizontally and sy vertically, and returns a reference to the matrix. *Nothing* there clearly states, at least to my reading, whether the "new" transform happens *before* or *after* any existing transforms that the QTransform is already doing. IMO, changing this to clarify that would help significantly. I would argue this should be done *even if* the class description explains this. However, I didn't find any clarification there, either. (Admittedly, I did not rigorously read the entire description, but that's still making my point; if it's hard to find, users are going to be confused.) -- Matthew ___ Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
Re: [Development] matrix math help needed - https://bugreports.qt.io/browse/QTBUG-84441
Matthew Woehlke (26 May 2020 18:15) > The documentation is not clear if the scale, rotate, etc. methods of > QTransform apply *before* or *after* whatever the QTransform is already > doing. The bug report indicates that they are applied *first*. > > Given the potential for breaking existing code which expects the current > behavior, my inclination would be to clarify the documentation to > clearly state the existing behavior. Yes, the docs do need updated; they do correctly say what QTransform does, but that behaviour managed to confuse me for a while, too. > (Note: I didn't actually test this myself or look at the code, I am just > going off of what the bug report says. In any case, however, the > documentation should be fixed.) Indeed. I did test this myself, and was surprised that double a = qDegreesToRadians(45.0); double sina = qSin(a); double cosa = qCos(a); QCOMPARE(QTransform().translate(50, 50).rotate(45).scale(0.5, 1.0), QTransform(0.5, 0, 0, 1.0, 0, 0) * QTransform(cosa, sina, -sina, cosa, 0, 0) * QTransform(1, 0, 0, 1, 50.0, 50.0)); passes. However, once it did, I knew what was going on ... Eddy. ___ Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
Re: [Development] matrix math help needed - https://bugreports.qt.io/browse/QTBUG-84441
On Tuesday, 26 May 2020 10:33:21 PDT Edward Welbourne wrote: > While I'm a linear algebraist ... and aware that the orthodoxy of > graphics programming does things "a bit differently" than linear > algebraists do, but allegedly for good reasons (that I might one day > learn). It's just the same thing written differently. [∞] ⎡ 0 1⎤=[8] ⎣-1 0⎦ -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel System Software Products ___ Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
Re: [Development] matrix math help needed - https://bugreports.qt.io/browse/QTBUG-84441
On 26/05/2020 13.33, Edward Welbourne wrote: It turns out that either way round works fine, as long as you're consistent. Which QTransform appears to be; and its docs correctly describe it the way it is. They are? It was *not at all* clear to me in what order the transforms would be applied. I will happily argue that the documentation in its current state is inadequate. I can also happily argue that that is, indeed, the only actual "bug" here. -- Matthew ___ Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
Re: [Development] matrix math help needed - https://bugreports.qt.io/browse/QTBUG-84441
On Tuesday, 26 May 2020 09:17:05 PDT Edward Welbourne wrote: >> (Matrix multiplication isn't commutative.) Thiago Macieira (26 May 2020 18:42) > This more or less summarises all I remember from matrix classes from high > school. While I'm a linear algebraist ... and aware that the orthodoxy of graphics programming does things "a bit differently" than linear algebraists do, but allegedly for good reasons (that I might one day learn). It's just the same thing written differently. >> I'll try to work out whether QMatrix / QTransform are consistent about it It seems QTransform was the only one in question here; and its matrices are the transposes of what I would have expected; hence its multiplication happens in the reverse of the order I expected. This is a self-consistent way to do things, just confusing to those who think they know how matrices work. It turns out that either way round works fine, as long as you're consistent. Which QTransform appears to be; and its docs correctly describe it the way it is. Eddy. ___ Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
Re: [Development] matrix math help needed - https://bugreports.qt.io/browse/QTBUG-84441
On Tuesday, 26 May 2020 09:17:05 PDT Edward Welbourne wrote: > (Matrix multiplication isn't commutative.) This more or less summarises all I remember from matrix classes from high school. > I'll try to work out whether QMatrix / QTransform are consistent about it Thanks, Eddy. -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel System Software Products ___ Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
Re: [Development] matrix math help needed - https://bugreports.qt.io/browse/QTBUG-84441
Thiago Macieira (26 May 2020 18:06) wrote: > Neither QMatrix nor QTransform have seen any change since the Qt 5.0 > repository import that was related to the underlying math. Only licensing > and C++ updates. > > The task https://bugreports.qt.io/browse/QTBUG-84441 is claiming there's > some wrong math or at least unexpected behaviour. Can someone who > understands these classes take a look? I'll comment. It's down to the confusing nature of what the order of operations is. (Matrix multiplication isn't commutative.) I'll try to work out whether QMatrix / QTransform are consistent about it ... Eddy. ___ Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
Re: [Development] matrix math help needed - https://bugreports.qt.io/browse/QTBUG-84441
On 26/05/2020 12.06, Thiago Macieira wrote: On Tuesday, 26 May 2020 08:59:09 PDT Thiago Macieira wrote: Neither QMatrix nor QTransform have seen any change since the Qt 5.0 repository import that was related to the underlying math. Only licensing and C++ updates. The task https://bugreports.qt.io/browse/QTBUG-84457 is claiming there's some wrong math or at least unexpected behaviour. Can someone who understands these classes take a look? Updated subject: https://bugreports.qt.io/browse/QTBUG-84441 is the right task. The documentation is not clear if the scale, rotate, etc. methods of QTransform apply *before* or *after* whatever the QTransform is already doing. The bug report indicates that they are applied *first*. Given the potential for breaking existing code which expects the current behavior, my inclination would be to clarify the documentation to clearly state the existing behavior. (Note: I didn't actually test this myself or look at the code, I am just going off of what the bug report says. In any case, however, the documentation should be fixed.) -- Matthew ___ Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
Re: [Development] matrix math help needed - https://bugreports.qt.io/browse/QTBUG-84441
On Tuesday, 26 May 2020 08:59:09 PDT Thiago Macieira wrote: > Neither QMatrix nor QTransform have seen any change since the Qt 5.0 > repository import that was related to the underlying math. Only licensing > and C++ updates. > > The task https://bugreports.qt.io/browse/QTBUG-84457 is claiming there's > some wrong math or at least unexpected behaviour. Can someone who > understands these classes take a look? Updated subject: https://bugreports.qt.io/browse/QTBUG-84441 is the right task. -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel System Software Products ___ Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
