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

Refactor PaintGraphCell #8897

Merged
merged 2 commits into from Mar 11, 2021
Merged

Conversation

mstv
Copy link
Member

@mstv mstv commented Feb 27, 2021

Proposed changes

  • Always restore the clip region and the SmoothingMode
  • Remove return value which is always true
  • Reduce if nesting
  • Replace var

Screenshots

N/A

Test methodology

  • review, manual and existing tests

Test environment(s)

  • Git Extensions 33.33.33
  • Build 11f8f3e
  • Git 2.27.0.windows.1 (recommended: 2.30.0 or later)
  • Microsoft Windows NT 10.0.19042.0
  • .NET Framework 4.8.4300.0
  • DPI 96dpi (no scaling)

✒️ I contribute this code under The Developer Certificate of Origin.

@mstv mstv self-assigned this Feb 27, 2021
@codecov
Copy link

codecov bot commented Feb 27, 2021

Codecov Report

Merging #8897 (11f8f3e) into master (2541a79) will increase coverage by 0.01%.
The diff coverage is 75.60%.

@@            Coverage Diff             @@
##           master    #8897      +/-   ##
==========================================
+ Coverage   56.06%   56.07%   +0.01%     
==========================================
  Files         922      922              
  Lines       65843    65842       -1     
  Branches    12073    12071       -2     
==========================================
+ Hits        36912    36924      +12     
+ Misses      25918    25914       -4     
+ Partials     3013     3004       -9     
Flag Coverage Δ
production 43.32% <75.60%> (+0.02%) ⬆️
tests 94.82% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@RussKie
Copy link
Member

RussKie commented Feb 28, 2021

Seems to work 👍

@mstv
Copy link
Member Author

mstv commented Mar 8, 2021

@drewnoakes, could you review, please?

Copy link
Member

@drewnoakes drewnoakes left a comment

Choose a reason for hiding this comment

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

Looks good.


int top = g.RenderingOrigin.Y;
Rectangle laneRect = new(0, top, width, rowHeight);
Region newClip = new(laneRect);
newClip.Intersect(oldClip);
g.Clip = newClip;
g.Clear(Color.Transparent);
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to use Clear vs. FillRegion? The latter would avoid needing to set/restore the Clip.

Copy link
Member Author

Choose a reason for hiding this comment

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

Clip is needed anyway in order to create the impression of drawn through graph lines by not letting end the lines at the row borders.

@mstv mstv merged commit aac9f60 into gitextensions:master Mar 11, 2021
@ghost ghost added this to the 3.6 milestone Mar 11, 2021
@mstv mstv deleted the refactor_graph_painting branch March 11, 2021 19:27
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.

None yet

3 participants