Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Layer "Scale dependent visibility" doesn't work anymore since 2.16 #23389

Closed
qgib opened this issue Aug 19, 2016 · 30 comments
Closed

Layer "Scale dependent visibility" doesn't work anymore since 2.16 #23389

qgib opened this issue Aug 19, 2016 · 30 comments
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter! High Priority Regression Something which used to work, but doesn't anymore Vectors Related to general vector layer handling (not specific data formats)
Milestone
Version 2.18

Comments

@qgib
Copy link
Contributor

qgib commented Aug 19, 2016 •
edited
Loading

Author Name: EDOUARD GOUYON (EDOUARD GOUYON)
Original Redmine Issue: 15463
Affected QGIS version: 2.18.4
Redmine category:vectors
Assignee: Sandro Santilli

Hi,
This is my first bug report here :) ! I Hope this will be efficient for all !

In my main GIS workspace all the layers initially set with a specific "Scale dependent visibility" on QGIS 2.12 are not working properly since QGIS 2.16:
For all those layers, on QGIS 2.16.1, the "Minimum (exclusive)" value is remaining OK (that is as initially set), but the "Maximum (inclusive)" value is now automatically and wrongly set to the value '1:100 000' instead of the value initially set. After trying to reset the good initial value and then applying with "apply" and "OK" buttons, the value always stays to '1:100 000' value.

When I try to open this main GIS workspace in QGIS 2.12 or 2.14, no problem, this max value of "Scale dependent visibility" is still OK.

Please see attachement file for a display comparison.

Thank you for your support.

Edouard Gouyon

Redmine related issue(s): 16601
@qgib
Copy link
Contributor Author

qgib commented Oct 17, 2016

Author Name: Matthias Kuhn (@m-kuhn)

Can you attach a .qgs project file with an affected layer?

@qgib
Copy link
Contributor Author

qgib commented Oct 17, 2016

Author Name: EDOUARD GOUYON (EDOUARD GOUYON)

Matthias Kuhn wrote:

Can you attach a .qgs project file with an affected layer?

Sorry I can't. Since The post of this bug, I've definitely downgraded to 2.14 version and the bug doesn't exists anymore. I didn't save any project with 2.16 in order not affect the good styling of my layers with the reported bug.

@qgib
Copy link
Contributor Author

qgib commented Oct 18, 2016

Author Name: Sandro Santilli (@strk)

Edouard if I read your original submission correctly we dont' need a project saved by 2.16 but by 2.12 ?

@qgib
Copy link
Contributor Author

qgib commented Oct 19, 2016

Author Name: Matthias Kuhn (@m-kuhn)

I assume that 1f1898d fixes this issue.

Basically, in 2.12, the scale "0" could be used for an infinitely max scale. In 2.16 1:1 was the max possible, anything else (like 0) would be set equal to the min scale (which could be 1:100'000), resulting in the layer never being visible.

Above commit fixes this for 2.18.

If someone is able to reproduce this with a recent nightly/2.18, please reopen.

  • status_id was changed from Open to Closed

@qgib
Copy link
Contributor Author

qgib commented Mar 4, 2017

Author Name: Jean-Gabriel JGH (Jean-Gabriel JGH)

Hello,

I am having the same issue with 2.18.3 and 2.18.4 (Windows 10, 64 bits), with both Shapefiles and Postgres layers.
I nailed it down a bit more than the OP. The issue with the "Maximum (inclusive)" occurs only for scales smaller than 1:100,000 (that is, 1:99,999 is fine but 1:100,001 will reset to 1:100,000). When one edits the scale, it is properly saved to the QGS file, under <maplayer minimumScale="100001" [...]>. However if you re-open the layer property, it is displayed as 1:100000 and this value (100000) will then be saved to the QGS file as if you purposely modified the scale.
Let's note that the same behavior occurs with rule based styling. Double clicking on a rule resets the Max Scale to 100,000.

As Matthias Kuhn has asked, please find a simple QGS file exhibiting the issue. It refers to a dummy layer (i.e. empty shapefile). This layer (dummylayer_ok) is set to have a max scale of 1:12,345 which works fine. A copy of the layer (dummylayer_reset), with a scale of 1:123,456 is there. You can see the scale in the QGS file but opening its property should show 1:100,000.

PS: I put an emphasis that the issue is, in the GUI, with the MAX scale, but the QGS file records this value as the MIN scale...

Let me know should you need any other information!
Jean-Gabriel

  • fixed_version_id removed Version 2.16
  • 10789 was configured as dummylayer.zip
  • 10790 was configured as minScaleReset.qgs
  • status_id was changed from Closed to Reopened

@qgib
Copy link
Contributor Author

qgib commented Mar 6, 2017

Author Name: Giovanni Manghi (@gioman)

Confirmed here too.

  • category_id was configured as Vectors
  • fixed_version_id was configured as Version 2.18
  • version was changed from 2.16.1 to 2.18.4

@qgib
Copy link
Contributor Author

qgib commented Apr 30, 2017

Author Name: Giovanni Manghi (@gioman)

  • regression was configured as 1

@qgib
Copy link
Contributor Author

qgib commented Apr 30, 2017

Author Name: Giovanni Manghi (@gioman)

  • priority_id was changed from Severe/Regression to High

@qgib
Copy link
Contributor Author

qgib commented Apr 30, 2017

Author Name: Giovanni Manghi (@gioman)

  • easy_fix was configured as 0

@qgib
Copy link
Contributor Author

qgib commented May 16, 2017

Author Name: Sandro Santilli (@strk)

  • description was changed from Hi,
    This is my first bug report here :) ! I Hope this will be efficient for all !

In my main GIS workspace all the layers initially set with a specific "Scale dependent visibility" on QGIS 2.12 are not working properly since QGIS 2.16:
For all those layers, on QGIS 2.16.1, the "Minimum (exclusive)" value is remaining OK (that is as initially set), but the "Maximum (inclusive)" value is now automatically and wrongly set to the value '1:100 000' instead of the value initially set. After trying to reset the good initial value and then applying with "apply" and "OK" buttons, the value always stays to '1:100 000' value.

When I try to open this main GIS workspace in QGIS 2.12 or 2.14, no problem, this max value of "Scale dependent visibility" is still OK.

Please see attachement file for a display comparison.

Thank you for your support.

Edouard Gouyon to Hi,
This is my first bug report here :) ! I Hope this will be efficient for all !

In my main GIS workspace all the layers initially set with a specific "Scale dependent visibility" on QGIS 2.12 are not working properly since QGIS 2.16:
For all those layers, on QGIS 2.16.1, the "Minimum (exclusive)" value is remaining OK (that is as initially set), but the "Maximum (inclusive)" value is now automatically and wrongly set to the value '1:100 000' instead of the value initially set. After trying to reset the good initial value and then applying with "apply" and "OK" buttons, the value always stays to '1:100 000' value.

When I try to open this main GIS workspace in QGIS 2.12 or 2.14, no problem, this max value of "Scale dependent visibility" is still OK.

Please see attachement file for a display comparison.

Thank you for your support.

Edouard Gouyon

  • assigned_to_id was configured as Sandro Santilli

@qgib
Copy link
Contributor Author

qgib commented May 16, 2017

Author Name: Sandro Santilli (@strk)

I confirm the issue.

Opening the project you attached (minScaleReset) with QGIS 2.14.10 and opening layer properties gives me:

  • dummylayer_reset: min 1:100,000,000 max 1:123,456
  • dummylayer_ok: min 1:100,000,000 max 1:12,345

Opening it with QGIS master (2.0.0-dev 4ed096b) gives:

  • dummylayer_reset: min 1:100,000,000 max 1:100,000
  • dummylayer_ok: min 1:100,000,000 max 1:12,345

I can still set (via GUI) that "max" value to 1:1,000,000, so there's no reason why 1:123,456 should not be accepted.

@qgib
Copy link
Contributor Author

qgib commented May 16, 2017

Author Name: Sandro Santilli (@strk)

Matthias 7 months ago you referenced 1f1898d but there's no such commit in the repository, can you remember which commit you were referring to ?

@qgib
Copy link
Contributor Author

qgib commented May 16, 2017

Author Name: Sandro Santilli (@strk)

Maybe it was 2475ee5 ?

@qgib
Copy link
Contributor Author

qgib commented May 16, 2017

Author Name: Sandro Santilli (@strk)

I confirm 2.18 branch as of 126cf5f also behaves the same as I reported above as happening in master (qgis-3).

@qgib
Copy link
Contributor Author

qgib commented May 23, 2017

Author Name: Sandro Santilli (@strk)

I take it back, the commit exists, just isn't recognized by this RedMine instance, looks like:
1f1898d

Same happens with the later:
2475ee5
2475ee5

@jef maybe you know what's going on here with commit references ?

@qgib
Copy link
Contributor Author

qgib commented May 23, 2017

Author Name: Jürgen Fischer (@jef-n)

@qgib
Copy link
Contributor Author

qgib commented May 23, 2017

Author Name: Sandro Santilli (@strk)

2.16 is also affected (at least as of final-2_16_3-106-g7ff0883)
Is it possible to turn the "Affected versions" into a multi-choice input ?
Or I think it makes sense to set it to the oldest affected branch, to make bisecting easier...

@qgib
Copy link
Contributor Author

qgib commented May 23, 2017

Author Name: Sandro Santilli (@strk)

I've run a git-bisect, finding that bd3fd74 is the first bad one. I still have to dig further (it's a merge commit for 4f18701)

@qgib
Copy link
Contributor Author

qgib commented May 23, 2017

Author Name: Sandro Santilli (@strk)

  • version was changed from 2.18.4 to 2.16.3
  • operating_system was changed from Windows to Windows, Linux

@qgib
Copy link
Contributor Author

qgib commented May 23, 2017

Author Name: Giovanni Manghi (@gioman)

no one will wver worok on 2.16, on the other hand 2.18 is the next release, knowing that LTR is THE affected version is MUCH more important.

  • version was changed from 2.16.3 to 2.18.4

@qgib
Copy link
Contributor Author

qgib commented May 23, 2017

Author Name: Sandro Santilli (@strk)

I filed a pull request for 2.18 branch: #4612
It's a oneliner, removing a line which looked suspicious in 4f18701
but I don't really understand why it fixes the issue yet, so it's important to test analyze further

I'll not start an "Affected QGIS version" semantic war here, please use the mailing list for that.

  • pull_request_patch_supplied was changed from 0 to 1

@qgib
Copy link
Contributor Author

qgib commented May 23, 2017

Author Name: Sandro Santilli (@strk)

The PR which introduced the regression, once merged: #2863

@qgib
Copy link
Contributor Author

qgib commented May 23, 2017

Author Name: Sandro Santilli (@strk)

Ok so full analysis is as follows:

  • When constructing a ScaleRangeWidget:
    • A signal handler is installed for scaleChanged signals emitted by minScaleWidget that places a limit on the maxScaleWidget
    • The max scale widget is given an arbitrary value of 1:100000, triggering the handler and thus setting a limit on maxScale
  • When opening layer properties panel, QgsScaleRangeWidget::setScaleRange is invoked, which tries to set maxScale before minScale, thus being limited by the previously set minScale.

So the proper fix here is to change setScalRange to set min before max.

It is arguable whether the arbitrary values to the range are worth keeping, but if we want to keep them it could be a better idea to use DBL_MIN/DBL_MAX for the limit...

@qgib
Copy link
Contributor Author

qgib commented May 23, 2017

Author Name: Sandro Santilli (@strk)

New PR replacing the old one: #4613

@qgib
Copy link
Contributor Author

qgib commented May 26, 2017

Author Name: Sandro Santilli (@strk)

Applied in changeset 55ffbf5.

  • status_id was changed from Reopened to Closed
  • done_ratio was changed from 0 to 100

@qgib
Copy link
Contributor Author

qgib commented May 26, 2017

Author Name: Sandro Santilli (@strk)

Reopening to make sure it's also fixed in master branch (3.0.0-dev) - and I'm still working on the testcase. Will send a new PR for master.

  • status_id was changed from Closed to Reopened

@qgib
Copy link
Contributor Author

qgib commented May 26, 2017

Author Name: Sandro Santilli (@strk)

PR with testcase for master: #4633

@qgib
Copy link
Contributor Author

qgib commented May 26, 2017

Author Name: Sandro Santilli (@strk)

PR with testcase for 2.18 branch : #4634

@qgib
Copy link
Contributor Author

qgib commented May 26, 2017

Author Name: Sandro Santilli (@strk)

  • status_id was changed from Reopened to Feedback

@qgib
Copy link
Contributor Author

qgib commented May 26, 2017

Author Name: Sandro Santilli (@strk)

Applied in changeset c0531bc.

  • status_id was changed from Feedback to Closed

@qgib qgib closed this as completed May 26, 2017
@qgib qgib added Bug Either a bug report, or a bug fix. Let's hope for the latter! High Priority Vectors Related to general vector layer handling (not specific data formats) Regression Something which used to work, but doesn't anymore labels May 25, 2019
@qgib qgib added this to the Version 2.18 milestone May 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter! High Priority Regression Something which used to work, but doesn't anymore Vectors Related to general vector layer handling (not specific data formats)
Projects
None yet
Milestone
Version 2.18
Development

No branches or pull requests
1 participant
@qgib