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

Blitter line drawing improvements #7082

Merged
merged 3 commits into from Jan 24, 2019

Conversation

@JGRennison
Copy link
Contributor

@JGRennison JGRennison commented Jan 22, 2019

  1. Change DrawLine to be templated
    This removes per-pixel overheads due to use of the SetPixel virtual method.
    These overheads included:
  • expensive virtual method call which prevents inlining
  • palette lookup for every pixel
  • branch on whether palette animation is enabled on every pixel
  1. Avoid signed overflow when drawing long lines

  2. Adjust line-drawing algorithm to reduce wasted off-screen work
    This clips the line segment to be within the screen area prior to pixel iteration.

The performance improvement of these changes is quite noticeable when many link graph overlay lines are being rendered.

@JGRennison JGRennison force-pushed the blitter-draw-line-improvements branch 2 times, most recently from 1b13a66 to cc9af4d Jan 22, 2019
@@ -7,15 +7,18 @@
* See the GNU General Public License for more details. You should have received a copy of the GNU General Public License along with OpenTTD. If not, see <http://www.gnu.org/licenses/>.
*/

/** @file base.cpp Implementation of the base for all blitters. */
/** @file base.hpp Common functionality for all blitter implemenytations. */
Copy link
Member

@PeterN PeterN Jan 24, 2019

Choose a reason for hiding this comment

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

base.hpp -> common.hpp

@PeterN
Copy link
Member

@PeterN PeterN commented Jan 24, 2019

I found it difficult to measure the performance benefit of this. Using a busy savegame, the average ms per tick in the graphics rendering is pretty much the same (well within noise tolerance) between this PR and master. However over a 4 month period (showing the exact same view), master was 2 game days behind this PR.

JGRennison added 3 commits Jan 24, 2019
This is remove per-pixel overheads due to use of the SetPixel virtual
method.
These overheads included:
* expensive virtual method call which prevents inlining
* palette lookup for every pixel
* branch on whether palette animation is enabled on every pixel

Regenerate project files.
…off-screen work

This clips the line segment to be within the screen area prior to pixel iteration.
@JGRennison
Copy link
Contributor Author

@JGRennison JGRennison commented Jan 24, 2019

I used this OpenTTDCoop savegame to test this: https://wiki.openttdcoop.org/ProZone:Archive_-_Games_11_-_20#gameid_13
As trains run with no orders it generates a very large number of linkgraph links.
Without these changes, after about 8 game days after it is loaded, turning on link graph overlay lines when looking at the station area on the south-west edge adds about 100 ms to the per-frame word viewports rendering time as measured by the fps window. This divides the frame rate by about 4 relative to having link graph overlay lines disabled.
With these changes applied under the same conditions, only about 4 ms are added to the per-frame world viewports rendering time as measured by the fps window, the decrease in frame rate is modest (a reduction of about 2 fps).

@JGRennison JGRennison force-pushed the blitter-draw-line-improvements branch from cc9af4d to 5deee94 Jan 24, 2019
@PeterN
Copy link
Member

@PeterN PeterN commented Jan 24, 2019

I see this makes a huge difference when using 32bpp-anim!

PeterN
PeterN approved these changes Jan 24, 2019
@PeterN PeterN merged commit 4b256fe into OpenTTD:master Jan 24, 2019
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants