Skip to content

Add a benchmark program for kiva graphics contexts#647

Merged
jwiggins merged 1 commit into
masterfrom
feature/kiva-benchmark
Mar 1, 2021
Merged

Add a benchmark program for kiva graphics contexts#647
jwiggins merged 1 commit into
masterfrom
feature/kiva-benchmark

Conversation

@jwiggins
Copy link
Copy Markdown
Member

@jwiggins jwiggins commented Mar 1, 2021

This is a small program which collects timing information for key drawing methods of GraphicsContext for each [available] backend and then optionally generates a report in HTML format.

I'd like to continue to improve this program, but I wanted to keep this initial PR under 500 LOC. Ideas for future improvements:

  • Add hyperlinks for the image outputs written by each benchmark to the timings table
  • "Benchmark" the non-UI backends to get a clearer picture of feature coverage.
  • Add a GitHub action which generates this report and adds it to the documentation site so that we can get an idea about relative performance over time.
  • Don't ignore the GL backend.

Copy link
Copy Markdown
Contributor

@rahulporuri rahulporuri left a comment

Choose a reason for hiding this comment

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

LGTM with one bikeshedding-style comment. I tested this locally (with and without the output directory) and it works as expected.

Ideas for future improvements:

Can we create a separate issue to keep track of these improvements?

Comment thread enable/gcbench/bench.py
import numpy as np

_MAX_DURATION = 1.0
_SIZE = (512, 512)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we be working with larger screen sizes e.g. (1024, 1024) given that we are regularly working with larger screens now? Not sure how much of an effect that will have on the benchmarking.

Copy link
Copy Markdown
Member Author

@jwiggins jwiggins Mar 1, 2021

Choose a reason for hiding this comment

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

Yeah... Doing this might lead to better results since you're drawing more pixels and spending more time in lower-level non-Kiva code. Only downside I see is that it results in larger images which we plan to upload along with the HTML output (when adding benchmarking results to documentation). We maybe don't need to worry about it too much. Images can be made smaller by making them easier to compress ("less noise").

@rahulporuri
Copy link
Copy Markdown
Contributor

rahulporuri commented Mar 1, 2021

The text rendering stands out to me (On Windows + PyQt5)

celiagg agg qpainter
celiagg show_text 2x kiva agg show_text 2x qpainter show_text 2x

@jwiggins
Copy link
Copy Markdown
Member Author

jwiggins commented Mar 1, 2021

The text rendering stands out to me

Whoa. That QPainter result looks wrong (why no space between lines?).

Ah, but the color issue is related to #648. There's confusion about whether stroke color or fill color applies to text. I'm tempted to just make whatever AGG does the "correct" way and make sure the other backends match.

@jwiggins
Copy link
Copy Markdown
Member Author

jwiggins commented Mar 1, 2021

I'm curious how Celiagg is ending up with a sans-serif font...

@jwiggins
Copy link
Copy Markdown
Member Author

jwiggins commented Mar 1, 2021

I'll open issues for the other stuff. Merging now. Thanks for the review

@jwiggins jwiggins merged commit 8c6632e into master Mar 1, 2021
@jwiggins jwiggins deleted the feature/kiva-benchmark branch March 1, 2021 15:38
@rahulporuri
Copy link
Copy Markdown
Contributor

There's confusion about whether stroke color or fill color applies to text. I'm tempted to just make whatever AGG does the "correct" way and make sure the other backends match.

I found the non-black color surprising because I thought Black fill color would be the default. I'm happy with QPainter overall - apart from the issue with space between lines.

I'm curious how Celiagg is ending up with a sans-serif font...

Isn't this a known issue with font usage on windows + celiagg?

@jwiggins
Copy link
Copy Markdown
Member Author

jwiggins commented Mar 1, 2021

Isn't this a known issue with font usage on windows + celiagg?

Yes, but I would still expect celiagg to use the same font as kiva.agg

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