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

Rendering speedup #1020

Closed
wants to merge 4 commits into from
Closed

Rendering speedup #1020

wants to merge 4 commits into from

Conversation

jekhor
Copy link
Contributor

@jekhor jekhor commented Dec 8, 2013

Reduce rendering time for polygons, lines and simple markers by eliminating overhead at coordinate transformation, polygons and lines clipping and simple marker rendering.

I get perfomance gain upto 19% for polygons and 15% for markers at big layers (Belarus dump of OSM).

A QVector::clear() method completely reinitializes the object. Calling
a QVector::reserve() before clear() is useless and causes big overhead at
array append operations.

Give ~6% perfomance gain.

Signed-off-by: Yauhen Kharuzhy <jekhor@gmail.com>
Clipping algorithm is expensive and can take a lot of time with layer
contained big amount of polygons (upto ~15% in my tests). Most of the
polygons ususally are inside clipping rectangle and simple check for all
points of polygon can significally reduce render time.

This will increase overhead when a lot of polygons cross borders of
clipping rectangle but such cases are rare.

Signed-off-by: Yauhen Kharuzhy <jekhor@gmail.com>
expression( "..." ) call create QString object at every invocation. For
layers with simple markers this overhead take upto 10% of rendering time.

Move expression finding to startRender() method.

Signed-off-by: Yauhen Kharuzhy <jekhor@gmail.com>
Every call of transformCoords() Qstring dir object is created. It uses
only for output information about errors, so move its creation to error
handling block.

Perfomance gain is ~7%.

Signed-off-by: Yauhen Kharuzhy <jekhor@gmail.com>
@mhugent
Copy link
Contributor

mhugent commented Dec 9, 2013

Assigned to @mhugent

Cool, can't wait to test that with my WMS benchmark here.

@ghost ghost assigned mhugent Dec 9, 2013
@ahuarte47
Copy link
Contributor

Hi jekhor, great job!

But I would like to notify that your last commit overlaps similar code in other PR #980.

LINE 163: https://github.com/ahuarte47/QGIS/blob/affd78a86da06c0e5cc558acdf0e6a2b3bca3c20/src/core/symbology-ng/qgsrendererv2.cpp

If both pull request are merged, I suppose that someone must be aware.

@jekhor
Copy link
Contributor Author

jekhor commented Dec 9, 2013

Hm, your version looks better than mine :) Commit 6618072 can be dropped.

@ahuarte47
Copy link
Contributor

I hope both pull requests are merged
Best regards

@gioman
Copy link
Contributor

gioman commented Dec 9, 2013

yeah, maybe is time to merge Alvaro's work (I haven't received any negative feedback so far, regarding regressions, by the people have tested the installer with Alvaro's patch).

@mhugent
Copy link
Contributor

mhugent commented Dec 9, 2013

The first three commits are pushed to master. Thanks, jekhor!

@gioman: Alvaro's merge is more difficult, because there, it is not in all cases possible to have the same result as without optimisation.

@mhugent mhugent closed this Dec 9, 2013
@ahuarte47
Copy link
Contributor

Hi @mhugent, Sorry, I don't understand your reply, the patch #980 optimizes the rendering speed about ~3x times for far zooms as commented in 'http://hub.qgis.org/issues/8725'

Can you please elaborate a bit more the objections? What do you think that I must rewrite ?
Thanks!

@gioman
Copy link
Contributor

gioman commented Dec 10, 2013

@mhugent I also does not understand your reply. With Alvaro's patch the default simplification options are always "safe". Moreover the simplification can be disabled in the general options and at layer level. At layer level you can choose to push the simplification to very high levels, and only in this case (but never at high scales) you can get "strange" results. Anyway this is a edge behavior that can be documented.

@ahuarte47
Copy link
Contributor

Thanks Giovanni, I could not express better :-)

@ahuarte47
Copy link
Contributor

I know that I edited the core of rendering, and I am a newcomer in QGIS, It seems dangerous :-), but IMHO I think need to improve the speed of painting in these situations, QGIS is a fantastic app, but there is a real test for a large layer...

Rustic parcels of cadastre of navarra (Polygon2D of 472658 records, 458mb)
Soft/hard: WindowsXP+SP3, 32bits, 4gb RAM.

Please, from your experience, I'm looking to improve in this patch that is necessary!

@nyalldawson
Copy link
Collaborator

I think @mhugent may be referring to:
#927 (comment) (the first dot point) and
#927 (comment)

@ahuarte47 I think ideally your improvements would be enabled by default without any visible quality loss in rendering. This might mean sacrificing some of the possible speed improvements for rendering quality. Maybe something like only activating the non anti-aliased rendering only when the simplification slider is increased a notch may work?

This is just my interpretation of the previous discussions - @mhugent or @wonder-sk should be able to clarify this.

@ahuarte47
Copy link
Contributor

Thanks Nyall, it is a great idea!, I can execute AA-disabling for '1-pixel geometries' only when simplification slider is greater than minimum value.

Now, as recommended @Matthias-Kuhn me, the simplification is executed using a flag of three values (Geometry simplification, envelope simplification and AA-disabling ) and an extra simplification factor. Now the patch uses the three simplifications.

What you think to change the discussion to http://hub.qgis.org/issues/8725 ?

http://hub.qgis.org/issues/8725#note-59 shows a test table using AA-disabling
Best regards

@mhugent
Copy link
Contributor

mhugent commented Dec 10, 2013

@ahuarte47: I like the rendering speedup by geometry simplification very much. The meaning of my sentence was more that we should not hurry and think clearly about all possible situations. Unfortunately, my December is quite busy and I did not have as much time to look at the changes as I wish. Sorry, my fault.

One thing that I'm curious about is how it is with special renderer and symbol layer types. E.g. say we have a simple line symbol layer drawn with a perpendicular offset. It could be that in the line geometry, two vertices are less than a pixel away from each other, but in the offset line, they are more than a pixel away.
Is this currently considered in the code?

@ahuarte47
Copy link
Contributor

Hi @mhugent, thanks for your reply.
You are right and I am aware of it and this patch must be checked thoroughly!

But the code is very simple, ignore points in device coordinates with a distance less of '1-pixel'. I will test situation that you say. Thanks!

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

5 participants