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.

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

fix wrong scale method with constant expressions #2528

Closed
wants to merge 1 commit into from

Conversation

vmora
Copy link
Contributor

@vmora vmora commented Dec 2, 2015

fix #13571

@nyalldawson
Copy link
Collaborator

@vmora just looking into what's changed with the font marker (causing the test to fail). I think QgsFontMarkerSymbolLayerV2 is missing a lot of the scale method handling that the other marker classes have, eg cloning isn't copying the scale method, and create/properties don't save & restore the scale method either.

Is the scale method stuff all effectively deprecated now? Is it only kept for compatibility with older projects?

@vmora
Copy link
Contributor Author

vmora commented Dec 3, 2015

@nyalldawson it's kept for public API compatibility. I shouldn't have touched the default, because botomline it breaks public API. I'll fix that right away.

BTW do you know if there is either 1) a way to have all test pass or 2) a target that run the tests that must pass ?

@nyalldawson
Copy link
Collaborator

So I guess if it's deprecated anyway it's not an issue that the font marker doesn't handle them correctly?

Anyway, I did think something was odd when I added the font marker test since I had to use such huge sizes to get the data defined size to work, but just assumed this was normal behaviour for the symbol. Guess I should have dug deeper at the time! I'll manually push this commit along with a fixed font marker test so that Travis doesn't complain.

1 similar comment
@nyalldawson
Copy link
Collaborator

So I guess if it's deprecated anyway it's not an issue that the font marker doesn't handle them correctly?

Anyway, I did think something was odd when I added the font marker test since I had to use such huge sizes to get the data defined size to work, but just assumed this was normal behaviour for the symbol. Guess I should have dug deeper at the time! I'll manually push this commit along with a fixed font marker test so that Travis doesn't complain.

@vmora
Copy link
Contributor Author

vmora commented Dec 3, 2015

I'll manually push this commit along with a fixed font marker test so that Travis doesn't complain

Thanks.

@nyalldawson
Copy link
Collaborator

Done in 701d970

@nyalldawson nyalldawson closed this Dec 3, 2015
@vmora
Copy link
Contributor Author

vmora commented Dec 4, 2015

Is the scale method stuff all effectively deprecated now? Is it only kept for compatibility with older projects?

No, the compatibility with older project is taken care of by conversion to equivalent scale expression (the scale method is set to linear then). If you use the GUI, the method should always be linear (modulo this bug).

The reason to keep the scale method is purely for public API compatibility (python code). Should be removed asap in 3.x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants