Bug 128399

Created attachment 16423 [details]
Proposed patch

Fxies this bug and also cleans up several other problems related to lines
maintaining their aspecr ratio when resizing with the SHIFT key pressed.

Do we want this for the upcoming release or should I add it to the branch?

  Anything that builds on the "aspect ratio preservation" code needs to stay 
in that branch... it doesn't make sense elsewhere.  I wonder why lines and 
arrows have this at all though.  It goes entirely against the purpose of 
lines and arrows.  They're supposed to stretch and move along with plots and 
labels, and they're supposed to do this relative to the contents rect of the 
endpoints parents.  I think this is making everything very complicated for no 
tangible benefit.

  Right now, the only object I can think of that should be preserving it's 
ratios is picture, with a "possibly" for boxes and ellipses.  (Does anyone 
even use ellipse?)

  In practice, the only boxes I've ever seen used have been to contain plots 
and labels, which means preserving aspect ratio would result in the wrong 
thing.

The patch was written to fix the bug. As a result it also correctly preserves aspect ratio if the user resizes holding the shift key down. Do we want to ship with a known crash?

  None of that code is in trunk as far as I can see.

I'm not sure what that means. The crash is in trunk.

  Doesn't crash for me.

  Ah now I see it.  I'm tempted to back out more of this code as opposed to 
pile in more complications.  I really don't like this.  (And it's simply a 
divide by zero so the fix should be very simple.)

This is part of the mouse UI only.  Once the modifier is released, the object 
can re-scale as before.

- can anyone think of when one might want this?  Personally, I have neither 
known it existed, nor wanted it.

That being said, we shouldn't ship with a know crash.

On Saturday 03 June 2006 08:30, George Staikos wrote:
[bugs.kde.org quoted mail]

Seems like this is all that's needed.  I'd rather not see more changes than 
necessary.  The existence of this code and crash alone are proof of the value 
of doing minimal changes.

@@ -861,7 +861,7 @@
       fromPoint = line->from();
       if (shift) {
         double absAspect = fabs(aspect);
-        if (absAspect < 500 && (double(abs((pos.y() - toPoint.y())/(pos.x() - 
toPoint.x()))) < aspect || absAspect < 0.1)) {
+        if (absAspect < 500 && (double(abs((pos.y() - fromPoint.y())/(pos.x() 
- fromPoint.x()))) < aspect || absAspect < 0.1)) {
           toPoint = QPoint(pos.x(), fromPoint.y() + int(aspect*(pos.x() - 
fromPoint.x())));
         } else {
           toPoint = QPoint(fromPoint.x() + int((pos.y() - 
fromPoint.y())/aspect), pos.y());

On Saturday 03 June 2006 12:34, netterfield@astro.utoronto.ca wrote:
> - can anyone think of when one might want this?  Personally, I have neither
> known it existed, nor wanted it.


  I really would like to see all this mouse handling re-simplified.  It seems 
to have grown without vision for the past year and it's horribly 
overcomplicated now.  It's not surprising that random bugs are popping up in 
there.  Simplifying means looking at the big picture again, and also removing 
questionable features.

SVN commit 549034 by staikos:

fix apparent C&P bug
BUG: 128399


 M  +1 -1      ksttoplevelview.cpp  


--- trunk/extragear/graphics/kst/src/libkstapp/ksttoplevelview.cpp #549033:549034
@@ -861,7 +861,7 @@
       fromPoint = line->from();
       if (shift) {
         double absAspect = fabs(aspect);
-        if (absAspect < 500 && (double(abs((pos.y() - toPoint.y())/(pos.x() - toPoint.x()))) < aspect || absAspect < 0.1)) {
+        if (absAspect < 500 && (double(abs((pos.y() - fromPoint.y())/(pos.x() - fromPoint.x()))) < aspect || absAspect < 0.1)) {
           toPoint = QPoint(pos.x(), fromPoint.y() + int(aspect*(pos.x() - fromPoint.x())));
         } else {
           toPoint = QPoint(fromPoint.x() + int((pos.y() - fromPoint.y())/aspect), pos.y());