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

Use QML to render points #443

Merged
merged 17 commits into from Jan 23, 2019

Conversation

Projects
None yet
3 participants
@3nids
Copy link
Member

3nids commented Jan 21, 2019

This adds a new GeometryRenderer item to render geometries.
Points are rendered using QML rectangles items, while line/polygons are forwarded to to cpp to be rendered using QSGNode.

This solves issue of points being highlighted as 1 pixel only (on non-openGL only?)

It is now used both for the locator results and the multi selection highlighter.

Fixes #435
Fixes #156

@3nids 3nids requested a review from m-kuhn Jan 21, 2019

Show resolved Hide resolved src/core/qgssggeometry.cpp Outdated
@m-kuhn

This comment has been minimized.

Copy link
Member

m-kuhn commented Jan 21, 2019

Good stuff.

I am not sure I completely get, how the geometry wrapper works.
Is a new wrapper created from a new geometry and then passed on and deleted later on and the wrapped geometry never changes?
In this case I would assume we could just pass the geometry (gadget) itself around and not need a QObject based subclass like the wrapper.
I would imagine a wrapper to be instantiated once (in QML) and then the wrapped geometry property inside changed.

@3nids

This comment has been minimized.

Copy link
Member Author

3nids commented Jan 22, 2019

Indeed, I am replacing the wrapper.

Mainly because in the locator filter (see https://github.com/opengisch/QField/pull/443/files#diff-40e6fb81d790b248689e34b3bc7724c6R170), I am doing this:

QgsGeometryWrapper *geomwrapper = new QgsGeometryWrapper( geom, layer->crs() );
mLocatorBridge->locatorHighlight()->setProperty( "geometry", QVariant::fromValue<QgsGeometryWrapper *>( geomwrapper ) );

I think we cannot use the basic QgsGeometry (gadget) because it needs to be a property of a QML item.

I started to implement the change to changing the properties of the wrapper instead of changing the wrapper, but it brings a bit more code and makes things less clear. I can go on this tomorrow, I'm just not sure what's the best approach.

@m-kuhn

This comment has been minimized.

Copy link
Member

m-kuhn commented Jan 22, 2019

I think we cannot use the basic QgsGeometry (gadget) because it needs to be a property of a QML item.

You can set gadgets as properties, I think no problem

@3nids

This comment has been minimized.

Copy link
Member Author

3nids commented Jan 22, 2019

ok, now using the properties of the wrapper instead of resetting the wrapper itself

@3nids 3nids force-pushed the point_as_circle branch from 69bc5df to bd3feb8 Jan 22, 2019

@m-kuhn

m-kuhn approved these changes Jan 23, 2019

@m-kuhn

This comment has been minimized.

Copy link
Member

m-kuhn commented Jan 23, 2019

🎉

@3nids 3nids force-pushed the point_as_circle branch from bd3feb8 to 058a5a8 Jan 23, 2019

@qfield-fairy

This comment has been minimized.

Copy link
Collaborator

qfield-fairy commented Jan 23, 2019

Uploaded test apks for armv7 and x86

@3nids 3nids merged commit 5e279b5 into master Jan 23, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@3nids 3nids deleted the point_as_circle branch Jan 23, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment