Skip to content

Conversation

spdr870
Copy link
Member

@spdr870 spdr870 commented Nov 15, 2018

Fixes #5782

Changes proposed in this pull request:

  • Example implementation for reducing useless curves at nearly 0 performance cost

First commit, remove the not-needed curves:
image

Second commit, make the lines more smooth:
image

What did I do to test the code and ensure quality:

  • Scrolling through the log

Has been tested on (remove any that don't apply):

  • GIT 2.19
  • Windows 10

@ghost ghost assigned spdr870 Nov 15, 2018
@ghost ghost added the status: ready label Nov 15, 2018
@codecov
Copy link

codecov bot commented Nov 15, 2018

Codecov Report

Merging #5783 into master will decrease coverage by <.01%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #5783      +/-   ##
==========================================
- Coverage   36.94%   36.93%   -0.01%     
==========================================
  Files         620      620              
  Lines       47416    47436      +20     
  Branches     6323     6330       +7     
==========================================
+ Hits        17518    17522       +4     
- Misses      29115    29132      +17     
+ Partials      783      782       -1

@spdr870 spdr870 force-pushed the feature/5782 branch 4 times, most recently from 2c8de70 to 1e51ebc Compare November 17, 2018 19:33
}
// Anti-aliasing with bezier & PixelOffsetMode.HighQuality
// introduces an offset of ~1/8 px - compensate it.
g.SmoothingMode = SmoothingMode.AntiAlias;
Copy link
Member

Choose a reason for hiding this comment

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

The comment still exists, but I don't see the 1/8px offset in code.

{
// Anti-aliasing with bezier & PixelOffsetMode.HighQuality
// introduces an offset of ~1/8 px - compensate it.
g.SmoothingMode = SmoothingMode.AntiAlias;
Copy link
Member

Choose a reason for hiding this comment

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

Ditto.

@mstv
Copy link
Member

mstv commented Nov 18, 2018

Thank you, the graph looks much clearer now!

Two little things could be improved yet.

  1. The curves should always go exactly vertically through nodes, i.e. the ends of diagonal sections should be rounded so that they converge towards the vertical line, like at the green arrow in the screenshot. This will make outgoing and incoming curves symmetrical.
    grafik
  2. Maybe these one-row-bows can be avoided, too:
    grafik

@RussKie RussKie added this to the 3.0.1 milestone Nov 18, 2018
@gerhardol
Copy link
Member

@spdr870
What is remaining before this can be merged?

@sharwell sharwell added 📜 status: needs cla The author needs to update contributors.txt before reviewing or merging can proceed and removed 📜 status: needs cla The author needs to update contributors.txt before reviewing or merging can proceed labels Jan 23, 2019
@RussKie RussKie removed this from the 3.1.0 milestone Apr 7, 2019
@ghost ghost added this to the 3.3 milestone Sep 17, 2019
@RussKie RussKie removed this from the 3.3 milestone Sep 17, 2019
@GintasS
Copy link
Collaborator

GintasS commented Aug 6, 2021

This is an old PR and should be closed or set to a draft. @gerhardol

@RussKie RussKie closed this Aug 6, 2021
@gerhardol
Copy link
Member

Several of the changes in this PR has been included in PRs by @mstv. Can you summarize if something is not included?

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.

Readability revision graph
7 participants