Skip to content

Fix rect drawing with celiagg#317

Merged
jwiggins merged 2 commits into
masterfrom
fix/celiagg-fuzzy-borders
Jun 15, 2019
Merged

Fix rect drawing with celiagg#317
jwiggins merged 2 commits into
masterfrom
fix/celiagg-fuzzy-borders

Conversation

@jwiggins

@jwiggins jwiggins commented Sep 1, 2018

Copy link
Copy Markdown
Member

Component borders were a bit fuzzy when drawn with celiagg thanks to a very specific optimization which was made in the kiva agg backend.

This replicates that in the celiagg backend as well.

Component borders were a bit fuzzy when drawn with celiagg thanks to
a very specific optimization which was made in the kiva agg backend.
This replicates that in the celiagg backend as well.
@jwiggins

jwiggins commented Sep 1, 2018

Copy link
Copy Markdown
Member Author

This was mentioned by a user in celiagg/celiagg#43

@codecov-io

codecov-io commented Sep 1, 2018

Copy link
Copy Markdown

Codecov Report

Merging #317 into master will decrease coverage by 0.77%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #317      +/-   ##
==========================================
- Coverage   33.69%   32.91%   -0.78%     
==========================================
  Files         206      206              
  Lines       18305    18303       -2     
  Branches     2411     2415       +4     
==========================================
- Hits         6167     6024     -143     
- Misses      11747    11893     +146     
+ Partials      391      386       -5
Impacted Files Coverage Δ
kiva/celiagg.py 0% <0%> (ø) ⬆️
enable/null/image.py 0% <0%> (-85.72%) ⬇️
kiva/fonttools/afm.py 13.95% <0%> (-31.17%) ⬇️
kiva/fonttools/font_manager.py 40.92% <0%> (-14.54%) ⬇️
enable/component_editor.py 75% <0%> (-4.17%) ⬇️
enable/colors.py 21.73% <0%> (-3.27%) ⬇️
kiva/trait_defs/kiva_font_trait.py 73.43% <0%> (-3.13%) ⬇️
kiva/quartz/setup.py 0% <0%> (ø) ⬆️
kiva/agg/setup.py 0% <0%> (ø) ⬆️
enable/vtk_backend/vtk_window.py 0% <0%> (ø) ⬆️
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0215b4c...346fff7. Read the comment docs.

@corranwebster

Copy link
Copy Markdown
Contributor

Hmmm... Chaco also does a half-pixel adjustment here https://github.com/enthought/chaco/blob/master/chaco/plot_graphics_context.py#L28 but I don't know that we get that class unless we specifically ask for it.

I don't like the ad-hoc half-pixel shifts. Not saying that we shouldn't do it to keep backwards consistency, but it bugs me.

Beyond that the code LGTM.

Comment thread kiva/celiagg.py Outdated
if (not self.canvas_state.anti_aliased and
self.canvas_state.line_width in (0.0, 1.0) and
fabs(self.transform.shx) < 1e-3 and
fabs(self.transform.shx) < 1e-3):

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Typo: One of these should be self.transform.shy

@corranwebster

Copy link
Copy Markdown
Contributor

I'm happy for this to be merged with the typo corrected.

Comment thread kiva/celiagg.py Outdated

@corranwebster corranwebster left a comment

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.

Fixed typo. OK with this since it replicates current behaviour.

@corranwebster corranwebster mentioned this pull request Jun 15, 2019
@jwiggins jwiggins merged commit dd09efa into master Jun 15, 2019
@jwiggins jwiggins deleted the fix/celiagg-fuzzy-borders branch June 15, 2019 18:46
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.

3 participants