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

[style manager] new default symbols shipped with QGIS & add a set of pre-defined tags #3793

Merged
merged 4 commits into from
Nov 22, 2016

Conversation

nirvn
Copy link
Contributor

@nirvn nirvn commented Nov 21, 2016

This PR refreshed the default symbols shipped with QGIS to better reflect the cartographic powerhouse that the project has become. The new symbols are:
defaults

In addition to updating the symbols, this PR also improves the default style database creation by moving away from copying a pre-existing .db in favor of creating a .db when needed and importing symbols declared in an .xml file.

  • On the UX front, the main benefit is being able to ship with pre-defined tags and favorite flags for the default symbols.
  • On the developer side, it means we get rid of the requirement to maintain both an .xml and a binary .db in the source tree. We also say aurevoir to the symbol_xml2db.py script and the need to update the (obsolete) script every time we add a new feature to QgsStyle.

* @see createMemoryDB()
*/
bool createDB( const QString& filename );
/** Creates a temporary memory database
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add some spacing between a method and the subsequent comment?

* \return returns the success state of the database creation
* @see createMemoryDB()
*/
bool createDB( const QString& filename );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that should be createDb, acronyms have been updated to this style for 3.0

@@ -1526,6 +1555,11 @@ bool QgsStyle::importXml( const QString& filename )
QDomElement symbolsElement = docEl.firstChildElement( QStringLiteral( "symbols" ) );
QDomElement e = symbolsElement.firstChildElement();

// gain speed by re-grouping the INSERT statements in a transaction
char* query;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does that not need to be cleaned up after usage? (some kind of free)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@m-kuhn , yeah, it's free'ed in the runEmptyQuery() function.

@nirvn
Copy link
Contributor Author

nirvn commented Nov 22, 2016

@m-kuhn , @nyalldawson , above comments addressed. That's ready to merge unless other issues are raised.

@m-kuhn
Copy link
Member

m-kuhn commented Nov 22, 2016

Thanks. I think now it should go to the api change doc ;)

@nirvn
Copy link
Contributor Author

nirvn commented Nov 22, 2016

@m-kuhn , what part? The shipping of and .xml instead of .db as our mean to deliver default symbols?

@nirvn
Copy link
Contributor Author

nirvn commented Nov 22, 2016

(removed)

@@ -208,8 +208,29 @@ class QgsStyle : QObject
//! Changes ramp's name
bool renameColorRamp( const QString& oldName, const QString& newName );

//! Creates a temporary memory database
bool createMemoryDB();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change of this method name

Copy link
Contributor Author

@nirvn nirvn Nov 22, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@m-kuhn , that's not an API break, that is a new function, wasn't there prior to what will become 3.0.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, ok, forget about it then

Instead of copyign a pre-existing .db shipped with QGIS, create
a new .db and import an .xml file containing our default  symbols.

On the UX front, the main benefit is being able to ship with
pre-defined tags and favorite flags for the default symbols.

On the developer side, it means we get rid of the requirement
to maintain both an .xml and a binary .db in the source tree.
We also say aurevoir to the symbol_xml2db.py script and the
need to update the (obsolete) script every time we add a new
feature to QgsStyle.
Gains are significant, importing 100 symbols would take 2.86s,
but takes only 0.18s when using transaction.
@m-kuhn m-kuhn merged commit 8dbe6ea into qgis:master Nov 22, 2016
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.

2 participants