Autres/audacity
1
0
Fork 0
mirror of https://github.com/cookiengineer/audacity synced 2025-07-16 16:47:41 +02:00
Code Issues Releases Wiki Activity

Fix cross-platform one-pixel differences in line drawing...

Browse Source 
... Make all line drawing go through our AColor which wraps the wxDC
line-drawing functions.

Despite what wxWidgets documentation says, the functions still don't
consistently include the first point and omit the last point of a line.  I
observed inclusion of both on Mac, while on Windows, omission sometimes of the
first point instead of the last.

Discrepancies could be observed in 2.3.0 between Windows and Mac, using the
Window magnifier or command+alt+ +, zooming in closely on the ends of the
shadow lines below and right of tracks, or at the bottom-right corners of the
bevels drawn around vertical rulers.

So where there is an observable one-pixel difference of drawing between
platforms, there is the question, which was the intent when the drawing code
was written?  Should the coordinates be corrected by one or not?

I reviewed each case and used my judgment.

Most of the calls are in drawing rulers.  Major tick lines were drawn five
pixels long, and minor, three, on Mac.  I keep this behavior, but you might
argue for making them four and two.

On the other hand the drawing of ruler grid lines, which you can see in
the equalization and frequency analysis windows, did need a one-pixel correction
to avoid straying out of bounds.
This commit is contained in:
Paul Licameli 2018-11-17 09:25:26 -05:00
parent 2d144bfbc2
commit e6324e86ac
7 changed files with 79 additions and 68 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

77
src/AColor.cpp
View File

@ -117,51 +117,51 @@ void AColor::Arrow(wxDC & dc, wxCoord x, wxCoord y, int width, bool down)
}
//
// Draw a line while accounting for differences in wxWidgets versions
// Draw a line, inclusive of endpoints,
// compensating for differences in wxWidgets versions across platforms
//
void AColor::Line(wxDC & dc, wxCoord x1, wxCoord y1, wxCoord x2, wxCoord y2)
{
// As of 2.8.9 (possibly earlier), wxDC::DrawLine() on the Mac draws the
// last point since it is now based on the NEW wxGraphicsContext system.
// Make the other platforms do the same thing since the other platforms
// "may" follow they get wxGraphicsContext going.
const wxPoint points[] { { x1, y1 }, { x2, y2 } };
Lines( dc, 2, points );
}
// Draw lines, INCLUSIVE of all endpoints
void AColor::Lines(wxDC &dc, size_t nPoints, const wxPoint points[])
{
if ( nPoints <= 1 ) {
if (nPoints == 1)
dc.DrawPoint( points[0] );
return;
}
for (size_t ii = 0; ii < nPoints - 1; ++ii) {
const auto &p1 = points[ii];
const auto &p2 = points[ii + 1];
// As of 2.8.9 (possibly earlier), wxDC::DrawLine() on the Mac draws the
// last point since it is now based on the NEW wxGraphicsContext system.
// Make the other platforms do the same thing since the other platforms
// "may" follow they get wxGraphicsContext going.
// PRL: as of 3.1.1, I still observe that on Mac, the last point is
// included, contrary to what documentation says. Also that on Windows,
// sometimes it is the first point that is excluded.
#if defined(__WXMAC__) || defined(__WXGTK3__)
dc.DrawLine(x1, y1, x2, y2);
dc.DrawLine(p1, p2);
#else
bool point = false;
if (x1 == x2) {
if (y1 < y2) {
y2++;
dc.DrawPoint(p1);
if ( p1 != p2 ) {
dc.DrawLine(p1, p2);
}
else if (y2 < y1) {
y1++;
}
else {
point = true;
}
}
else if (y1 == y2) {
if (x1 < x2) {
x2++;
}
else if (x2 < x1) {
x1++;
}
else {
point = true;
}
}
else {
dc.DrawPoint(x2, y2);
#endif
}
if (point) {
dc.DrawPoint(x2, y2);
}
else {
dc.DrawLine(x1, y1, x2, y2);
}
#if defined(__WXMAC__) || defined(__WXGTK3__)
;
#else
dc.DrawPoint( points[ nPoints - 1 ] );
#endif
}
@ -264,6 +264,9 @@ void AColor::BevelTrackInfo(wxDC & dc, bool up, const wxRect & r, bool highlight
#ifndef EXPERIMENTAL_THEMING
Bevel( dc, up, r );
#else
// Note that the actually drawn rectangle extends one pixel right of and
// below the given
wxColour col;
col = Blend( theTheme.Colour( clrTrackInfo ), up ? wxColour( 255,255,255):wxColour(0,0,0));

7
src/AColor.h
View File

@ -63,7 +63,14 @@ class AColor {
static void ReInit();
static void Arrow(wxDC & dc, wxCoord x, wxCoord y, int width, bool down = true);
// Draw a line, INCLUSIVE of both endpoints
// (unlike what wxDC::DrawLine() documentation specifies)
static void Line(wxDC & dc, wxCoord x1, wxCoord y1, wxCoord x2, wxCoord y2);
// Draw lines, INCLUSIVE of all endpoints
static void Lines(wxDC &dc, size_t nPoints, const wxPoint points[]);
static void DrawFocus(wxDC & dc, wxRect & r);
static void Bevel(wxDC & dc, bool up, const wxRect & r);
static void Bevel2

4
src/AdornedRulerPanel.cpp
View File

@ -1913,9 +1913,9 @@ void AdornedRulerPanel::DoDrawEdge(wxDC *dc)
// Black stroke at bottom
dc->SetPen( *wxBLACK_PEN );
dc->DrawLine( mOuter.x,
AColor::Line( *dc, mOuter.x,
mOuter.y + mOuter.height - 1,
mOuter.x + mOuter.width,
mOuter.x + mOuter.width - 1 ,
mOuter.y + mOuter.height - 1 );
}

2
src/effects/AutoDuck.cpp
View File

@ -715,7 +715,7 @@ void EffectAutoDuckPanel::OnPaint(wxPaintEvent & WXUNUSED(evt))
points[5].x = clientWidth - 10;
points[5].y = DUCK_AMOUNT_START;
dc.DrawLines(6, points);
AColor::Lines(dc, 6, points);
dc.SetPen(wxPen(*wxBLACK, 1, wxPENSTYLE_DOT));

4
src/widgets/KeyView.cpp
View File

@ -1102,9 +1102,9 @@ KeyView::OnDrawBackground(wxDC & dc, const wxRect & rect, size_t line) const
{
// Non-selected lines get a thin bottom border
dc.SetPen(wxColour(240, 240, 240));
dc.DrawLine(r.GetLeft(), r.GetBottom(), r.GetRight(), r.GetBottom());
AColor::Line(dc, r.GetLeft(), r.GetBottom(), r.GetRight(), r.GetBottom());
if (mViewType == ViewByTree )
dc.DrawLine(r2.GetLeft(), r2.GetBottom(), r2.GetRight(), r2.GetBottom());
AColor::Line(dc, r2.GetLeft(), r2.GetBottom(), r2.GetRight(), r2.GetBottom());
}
}

6
src/widgets/Meter.cpp
View File

@ -523,13 +523,13 @@ void MeterPanel::OnPaint(wxPaintEvent & WXUNUSED(event))
{
// 2 pixel spacing between the LEDs
if( (i%7)<2 ){
dc.DrawLine( i+r.x, r.y, i+r.x, r.y+r.height );
AColor::Line( dc, i+r.x, r.y, i+r.x, r.y+r.height );
} else {
// The LEDs have triangular ends.
// This code shapes the ends.
int j = abs( (i%7)-4);
dc.DrawLine( i+r.x, r.y, i+r.x, r.y+j +1);
dc.DrawLine( i+r.x, r.y+r.height-j, i+r.x, r.y+r.height );
AColor::Line( dc, i+r.x, r.y, i+r.x, r.y+j +1);
AColor::Line( dc, i+r.x, r.y+r.height-j, i+r.x, r.y+r.height );
}
}
}

47
src/widgets/Ruler.cpp
View File

@ -58,6 +58,7 @@ array of Ruler::Label.
#include "Ruler.h"
#include "../AColor.h"
#include "../AllThemeResources.h"
#include "../NumberScale.h"
#include "../Theme.h"
@ -1307,19 +1308,19 @@ void Ruler::Draw(wxDC& dc, const TimeTrack* timetrack)
{
if (mOrientation == wxHORIZONTAL) {
if (mFlip)
mDC->DrawLine(mLeft, mTop, mRight+1, mTop);
AColor::Line(*mDC, mLeft, mTop, mRight, mTop);
else
mDC->DrawLine(mLeft, mBottom, mRight+1, mBottom);
AColor::Line(*mDC, mLeft, mBottom, mRight, mBottom);
}
else {
if (mFlip)
mDC->DrawLine(mLeft, mTop, mLeft, mBottom+1);
AColor::Line(*mDC, mLeft, mTop, mLeft, mBottom);
else
{
// These calculations appear to be wrong, and to never have been used (so not tested) prior to MixerBoard.
// mDC->DrawLine(mRect.x-mRect.width, mTop, mRect.x-mRect.width, mBottom+1);
// AColor::Line(*mDC, mRect.x-mRect.width, mTop, mRect.x-mRect.width, mBottom);
const int nLineX = mRight - 1;
mDC->DrawLine(nLineX, mTop, nLineX, mBottom+1);
AColor::Line(*mDC, nLineX, mTop, nLineX, mBottom);
}
}
}
@ -1341,18 +1342,18 @@ void Ruler::Draw(wxDC& dc, const TimeTrack* timetrack)
{
if (mOrientation == wxHORIZONTAL) {
if (mFlip)
mDC->DrawLine(mLeft + pos, mTop,
AColor::Line(*mDC, mLeft + pos, mTop,
mLeft + pos, mTop + 4);
else
mDC->DrawLine(mLeft + pos, mBottom - 4,
AColor::Line(*mDC, mLeft + pos, mBottom - 4,
mLeft + pos, mBottom);
}
else {
if (mFlip)
mDC->DrawLine(mLeft, mTop + pos,
AColor::Line(*mDC, mLeft, mTop + pos,
mLeft + 4, mTop + pos);
else
mDC->DrawLine(mRight - 4, mTop + pos,
AColor::Line(*mDC, mRight - 4, mTop + pos,
mRight, mTop + pos);
}
}
@ -1369,19 +1370,19 @@ void Ruler::Draw(wxDC& dc, const TimeTrack* timetrack)
if (mOrientation == wxHORIZONTAL)
{
if (mFlip)
mDC->DrawLine(mLeft + pos, mTop,
AColor::Line(*mDC, mLeft + pos, mTop,
mLeft + pos, mTop + 2);
else
mDC->DrawLine(mLeft + pos, mBottom - 2,
AColor::Line(*mDC, mLeft + pos, mBottom - 2,
mLeft + pos, mBottom);
}
else
{
if (mFlip)
mDC->DrawLine(mLeft, mTop + pos,
AColor::Line(*mDC, mLeft, mTop + pos,
mLeft + 2, mTop + pos);
else
mDC->DrawLine(mRight - 2, mTop + pos,
AColor::Line(*mDC, mRight - 2, mTop + pos,
mRight, mTop + pos);
}
}
@ -1401,19 +1402,19 @@ void Ruler::Draw(wxDC& dc, const TimeTrack* timetrack)
if (mOrientation == wxHORIZONTAL)
{
if (mFlip)
mDC->DrawLine(mLeft + pos, mTop,
AColor::Line(*mDC, mLeft + pos, mTop,
mLeft + pos, mTop + 2);
else
mDC->DrawLine(mLeft + pos, mBottom - 2,
AColor::Line(*mDC, mLeft + pos, mBottom - 2,
mLeft + pos, mBottom);
}
else
{
if (mFlip)
mDC->DrawLine(mLeft, mTop + pos,
AColor::Line(*mDC, mLeft, mTop + pos,
mLeft + 2, mTop + pos);
else
mDC->DrawLine(mRight - 2, mTop + pos,
AColor::Line(*mDC, mRight - 2, mTop + pos,
mRight, mTop + pos);
}
}
@ -1442,11 +1443,11 @@ void Ruler::DrawGrid(wxDC& dc, int length, bool minor, bool major, int xOffset,
gridPos = mMinorLabels[i].pos;
if(mOrientation == wxHORIZONTAL) {
if((gridPos != 0) && (gridPos != mGridLineLength))
mDC->DrawLine(gridPos+xOffset, yOffset, gridPos+xOffset, mGridLineLength+yOffset);
AColor::Line(*mDC, gridPos+xOffset, yOffset, gridPos+xOffset, mGridLineLength-1+yOffset);
}
else {
if((gridPos != 0) && (gridPos != mGridLineLength))
mDC->DrawLine(xOffset, gridPos+yOffset, mGridLineLength+xOffset, gridPos+yOffset);
AColor::Line(*mDC, xOffset, gridPos+yOffset, mGridLineLength-1+xOffset, gridPos+yOffset);
}
}
}
@ -1458,11 +1459,11 @@ void Ruler::DrawGrid(wxDC& dc, int length, bool minor, bool major, int xOffset,
gridPos = mMajorLabels[i].pos;
if(mOrientation == wxHORIZONTAL) {
if((gridPos != 0) && (gridPos != mGridLineLength))
mDC->DrawLine(gridPos+xOffset, yOffset, gridPos+xOffset, mGridLineLength+yOffset);
AColor::Line(*mDC, gridPos+xOffset, yOffset, gridPos+xOffset, mGridLineLength-1+yOffset);
}
else {
if((gridPos != 0) && (gridPos != mGridLineLength))
mDC->DrawLine(xOffset, gridPos+yOffset, mGridLineLength+xOffset, gridPos+yOffset);
AColor::Line(*mDC, xOffset, gridPos+yOffset, mGridLineLength-1+xOffset, gridPos+yOffset);
}
}
@ -1472,11 +1473,11 @@ void Ruler::DrawGrid(wxDC& dc, int length, bool minor, bool major, int xOffset,
mDC->SetPen(*wxBLACK_PEN);
if(mOrientation == wxHORIZONTAL) {
if(zeroPosition != mGridLineLength)
mDC->DrawLine(zeroPosition+xOffset, yOffset, zeroPosition+xOffset, mGridLineLength+yOffset);
AColor::Line(*mDC, zeroPosition+xOffset, yOffset, zeroPosition+xOffset, mGridLineLength-1+yOffset);
}
else {
if(zeroPosition != mGridLineLength)
mDC->DrawLine(xOffset, zeroPosition+yOffset, mGridLineLength+xOffset, zeroPosition+yOffset);
AColor::Line(*mDC, xOffset, zeroPosition+yOffset, mGridLineLength-1+xOffset, zeroPosition+yOffset);
}
}
}