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

Hillshade renderer speed improvements #7411

Merged
merged 5 commits into from Jul 19, 2018
Merged

Conversation

elpaso
Copy link
Contributor

@elpaso elpaso commented Jul 13, 2018

While working on OpenCL stuff I've got an advice
from Nyall and Even regarding some speed improvement
used by GDAL DEM, and here they are.

According to my measurements the speed gain is roughly:

  • 20% for unidirectional
  • 70% for multi directional

While working on OpenCL stuff I've got an advice
from Nyall and Even regarding some speed improvement
used by GDAL DEM, and here they are.

According to my measurements the speed gain is roughly:

+ 20% for unidirectional
+ 70% for multi directional
@elpaso elpaso requested a review from rouault July 13, 2018 15:16
@@ -264,6 +304,13 @@ QgsRasterBlock *QgsHillshadeRenderer::block( int bandNo, const QgsRectangle &ext
}
}

#ifdef QGISDEBUG
QgsMessageLog::logMessage( QStringLiteral( "CPU Rendering time for hillshade (%2 x %3 ): %4 ms" )
Copy link
Member

Choose a reason for hiding this comment

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

This should probably also depend on the rendering debug option being enabled.

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 do you mean settings.value( QStringLiteral( "Map/logCanvasRefreshEvent" ), false ).toBool() ?

@nyalldawson
Copy link
Collaborator

Nice!

@nyalldawson nyalldawson added the Optimization I feel the need... the need for speed! label Jul 14, 2018
@nyalldawson nyalldawson added this to the 3.4 milestone Jul 14, 2018
@nirvn
Copy link
Contributor

nirvn commented Jul 16, 2018

Nice -- while we're at it, I'm wondering whether we should add a "help/tip" label into the renderer's widget panel, which would state: "To avoid visual artifacts, set the zoomed in resambling to bilinear, and insure the project's CRS matches that of your raster".

The project CRS part if needed, the zoomed in resambling method is disabled for reprojected rasters.

The tip label is to help people hitting the issue/limitation below:
image

@nyalldawson
Copy link
Collaborator

@nyalldawson , @elpaso , I was re-thinking this, is it better to say "make sure project's CRS matches that of the raster", or "make sure the raster's CRS matches that of the project"? :)

@nirvn - let's move that discussion out of this PR. It's not related so shouldn't hold back this one.

@nirvn
Copy link
Contributor

nirvn commented Jul 17, 2018

@nyalldawson , sounds good.

@elpaso elpaso merged commit 1c9b65d into qgis:master Jul 19, 2018
@elpaso elpaso deleted the hillshade-fast branch July 19, 2018 06:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Optimization I feel the need... the need for speed!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants