Bug report #6194

QgsRectangle expand uses wrong factor

Added by Magnus Homann about 13 years ago. Updated about 13 years ago.

Status:Closed
Priority:Normal
Assignee:Magnus Homann
Category:-
Affected QGIS version:master Regression?:No
Operating System: Easy fix?:No
Pull Request or Patch supplied:No Resolution:
Crashes QGIS or corrupts data:No Copied to github as #:15508

Description

The expand( scaleFactor ) function in QgsRectangle does not work as expected with a scale factor. Patch and test case is on the way! :-)

History

#1 Updated by Magnus Homann about 13 years ago

  • File issue6194.patch added
  • Status changed from Open to Closed
  • Pull Request or Patch supplied changed from No to Yes

Here's a patch with test case, someone please review and commit?

#2 Updated by Magnus Homann about 13 years ago

  • Status changed from Closed to Reopened

#3 Updated by Tim Sutton about 13 years ago

I'm testing your patch and will apply afterwards. Thanks!

Tim

#4 Updated by Tim Sutton about 13 years ago

Hi

So just one issue - the qgsrect.sip expand method is missing.

/home/timlinux/dev/cpp/Quantum-GIS/build-qt-creator/python/core/sipcorepart2.cpp: In function ‘PyObject* meth_QgsRectangle_expand(PyObject*, PyObject*)’:
/home/timlinux/dev/cpp/Quantum-GIS/build-qt-creator/python/core/sipcorepart2.cpp:2513:21: error: ‘class QgsRectangle’ has no member named ‘expand’
make[2]: *** [python/CMakeFiles/python_module_qgis_core.dir/core/sipcorepart2.cpp.o] Error 1
make[1]: *** [python/CMakeFiles/python_module_qgis_core.dir/all] Error 2
make: *** [all] Error 2

#5 Updated by Magnus Homann about 13 years ago

There were two functions in QgsRectangle, scale() and expand(). The only difference is that expand() used scalefactor*2(!). So I tried performing some API cleanup, but forgot to remove the SIP (becasue I wasn't using bindings).

If touching API is a no-no, I will of course put the function back, else I propose a clean-up. Please advise. :-)

#6 Updated by Jürgen Fischer about 13 years ago

Magnus Homann wrote:

There were two functions in QgsRectangle, scale() and expand(). The only difference is that expand() used scalefactor*2(!). So I tried performing some API cleanup, but forgot to remove the SIP (becasue I wasn't using bindings).

If touching API is a no-no, I will of course put the function back, else I propose a clean-up. Please advise. :-)

You could deprecate expand() first - and still call scale() in it to cleanup. Is it correct that you double the scale in qgsidentifyresults.cpp and qgsmapcanvas.cpp, but divide it by two in qgsmaptoolzoom.cpp? BTW the latter change, obviously wasn't run through prepare-commit.sh

#7 Updated by Magnus Homann about 13 years ago

Thanks for the feedback! How do I mark a function as depreciated?

#8 Updated by Jürgen Fischer about 13 years ago

Magnus Homann wrote:

Thanks for the feedback! How do I mark a function as depreciated?

UTSM ;) We have a couple of deprecated functions already. Look for Q_DECL_DEPRECATED (C++) and /Deprecated/ (SIP).

#9 Updated by Magnus Homann about 13 years ago

  • Pull Request or Patch supplied changed from Yes to No

I've made a pull request on GitHub on this issue now. It's my first... :-)

#10 Updated by Magnus Homann about 13 years ago

  • File deleted (issue6194.patch)

#11 Updated by Magnus Homann about 13 years ago

  • Status changed from Reopened to Closed

Fix merged in b7e6e64f0ff85e4a54ff29a52b57409da3a6548d

Also available in: Atom PDF