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

Grid supports anti-aliasing #2864

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Asachoo
Copy link

@Asachoo Asachoo commented Oct 31, 2023

The grid created using the showGrid method will invoke the setGrid method of Axis to configure various properties of the grid. However, due to the default setting of anti-aliasing to False in the drawPicture method of Axisitem.py, the resulting grid often appears to have inconsistent thickness (even though the line width of QPen remains consistent).

def drawPicture(self, p, axisSpec, tickSpecs, textSpecs):
profiler = debug.Profiler()
p.setRenderHint(p.RenderHint.Antialiasing, False)
p.setRenderHint(p.RenderHint.TextAntialiasing, True)
## draw long line along axis
pen, p1, p2 = axisSpec
p.setPen(pen)
p.drawLine(p1, p2)
# p.translate(0.5,0) ## resolves some damn pixel ambiguity

In this PR, i have added a new property in the setStyle method to allow users to determine whether to enable anti-aliasing rendering for grid objects.

The following two pictures show the difference between whether the axis is set to anti-aliasing or not.

without anti-aliasing:
without

with anti-aliasing:
with_anti

After adding the anti-aliasing attribute, the entire grid does look clearer and neater.

Other Tasks

Bump Dependency Versions

Files that need updates

Confirm the following files have been either updated or there has been a determination that no update is needed.

  • README.md
  • setup.py
  • tox.ini
  • .github/workflows/main.yml and associated requirements.txt and conda environemt.yml files
  • pyproject.toml
  • binder/requirements.txt
Pre-Release Checklist

Pre Release Checklist

  • Update version info in __init__.py
  • Update CHANGELOG primarily using contents from automated changelog generation in GitHub release page
  • Have git tag in the format of pyqtgraph-
Post-Release Checklist

Steps To Complete

  • Append .dev0 to __version__ in __init__.py
  • Announce on mail list
  • Announce on Twitter

@Asachoo Asachoo changed the title Grid supports anti-aliasing to ensure that grid lines appear to have … Grid supports anti-aliasing Oct 31, 2023
@j9ac9k
Copy link
Member

j9ac9k commented Nov 2, 2023

Hi @Asachoo thanks for the PR

The diff looks good to me, is there a reason that you think that the grid should have anti-aliasing disabled by default? I know there is a performance penalty, but I'm wondering if this is a use-case where it might be worthwhile....

Do you have a hidpi monitor, and if so, do you know what scaling factor you are using? I wonder if this is one of those things where if you had non-integer based scaling vs. integer based scaling...

@Asachoo
Copy link
Author

Asachoo commented Nov 2, 2023

@j9ac9k Hi, thank you for reviewing the PR.

Regarding the first point, I actually introduced an optional parameter based on the existing default settings. In other words, setting anti-aliasing to False is the default option for this PR. After conducting a quick investigation, it seems that this default configuration has been in place for the past 12 years. Considering the longstanding acceptance of this option, I decided not to change the default configuration of disabling anti-aliasing. However, personally, I would prefer to have anti-aliasing enabled by default as I have found the performance impact to be minimal in my experience.

Regarding your second point, on my Windows laptop with a resolution of 1920*1080, the scaling factors of 100%, 125%, and 150% provide a consistent experience. However, I am unsure how it would perform on higher-resolution displays.

Please let me know if there's anything else or if further improvements are needed.

@j9ac9k
Copy link
Member

j9ac9k commented Nov 3, 2023

Hi @Asachoo

Thanks for your detailed response, I appreciate the effort to preserve existing behavior. I think in this case, we should change the default False option, not to True, but to inherit the antialias global option (which defaults to False).

You can see an example of it being used here:

antialias=pg.getConfigOption('antialias'),

@NilsNemitz
Copy link
Contributor

NilsNemitz commented Nov 3, 2023

A quick comment on why this is off by default and doesn't use the library anti-alias setting so far:

I believe I have seen the library code turn off anti-aliasing in multiple places for lines that are perfectly vertical or horizontal. My interpretation has been that this exactly tries to avoid the problem shown in @Asachoo 's demonstration images: With anti-aliasing on, a fractional position would draw over two pixel coordinates at reduced intensity.

Turning anti-aliasing off for these lines is (I believe) supposed to make sure they always draw aligned with one particular row or column of screen pixels. Effectively this used to trade a sub-pixel position error for a sharp and consistent appearance.

The demo images show that this does not seem to work in all cases anymore. Fractional scaling modes of 125% and 150% would be an obvious reason. If it even breaks in 100% mode now, then its clearly time to remove the special treatment of vertical and horizontal lines. But if it still works in non-scaled modes, then it might be good to keep the special status for now (until the majority of users have screens with more pixels than the eye can resolve).

Maybe a new config option for antialiasing the grid-aligned horizontal / vertical lines might be an option for a smooth transition? I expect that we'll start seeing this problem in multiple places.

@j9ac9k
Copy link
Member

j9ac9k commented Nov 3, 2023

@Asachoo

Can you share the code you used to generate the screenshots? I have access to hidpi/non-hidpi displays on both macOS and Windows (and could do Linux if needed too). I agree with @NilsNemitz that historically we've disabled anti-aliasing on vertical or horizontal lines for the reasons he stated, but if we're getting the behavior you're seeing across platforms; we clearly need to revisit the assumption that disabling anti-aliasing will make vertical and horizontal lines sharp/consistent.

@pijyoi
Copy link
Contributor

pijyoi commented Nov 3, 2023

As an example, #2709 disabled anti-aliasing for horizontal and vertical infinite lines.

I think perhaps the issue may have to do with the combination of opacity + anti-aliasing + hidpi?
Grid lines are normally drawn with less than 100% opacity, I think.

I did see some strangeness with the Auto button which changes its opacity whenever the mouse hovers over it.

@Asachoo
Copy link
Author

Asachoo commented Nov 6, 2023

@Asachoo

Can you share the code you used to generate the screenshots? I have access to hidpi/non-hidpi displays on both macOS and Windows (and could do Linux if needed too). I agree with @NilsNemitz that historically we've disabled anti-aliasing on vertical or horizontal lines for the reasons he stated, but if we're getting the behavior you're seeing across platforms; we clearly need to revisit the assumption that disabling anti-aliasing will make vertical and horizontal lines sharp/consistent.

I apologize for not submitting my test case code at the beginning. My test case has been tested on both Windows and Ubuntu. The pyqtgraph plotting is not directly performed in QMainWindow, but rather in a QMdiSubWindow created within QMdiArea (this information might be important, and I apologize for not mentioning it initially). Here is the extracted test code based on my project, which has been tested and can reproduce the issue on my machine (system, screen and packages summary: Ubuntu 22.04.3 LTS, resolution 1560*1440, scale 100%, PySide2 5.15.2.1, pyqtgraph 0.13.3).

import sys
import numpy as np
import pyqtgraph as pg
from PySide2.QtGui import *
from PySide2.QtCore import *
from PySide2.QtWidgets import *


class NetworkPlot(QWidget):
    def __init__(self, parent) -> None:
        super().__init__(parent)

        layout = QHBoxLayout(self)

        self.axis_font: QFont = QFont("Arial", 14, -1, False)
        self.curve = pg.PlotItem()
        self.curve.showGrid(True, True, alpha=1.0)

        # self.curve.setLabel(axis="bottom", text="Frequency", units="Hz")
        # self.curve.setLabel(axis="left", text="Decibels", units="db")

        pg.setConfigOption("background", "w")

        self.curve.getAxis("top").setStyle(showValues=False)
        self.curve.getAxis("right").setStyle(showValues=False)

        axis_pen = pg.mkPen(color="#000000", width=1.75)

        for orientation in ("top", "bottom", "left", "right"):
            self.curve.showAxis(orientation)
            self.curve.getAxis(orientation).setStyle(tickAlpha=0.5)
            self.curve.getAxis(orientation).setPen(axis_pen)

        for orientation in ("bottom", "left"):
            self.curve.getAxis(orientation).label.setFont(self.axis_font)
            self.curve.getAxis(orientation).setTextPen("#000000")
            self.curve.getAxis(orientation).setTickFont(self.axis_font)

        # Set Curve layout
        self.curve_view: pg.GraphicsLayoutWidget = pg.GraphicsLayoutWidget(self)
        self.curve_view.addItem(self.curve)

        layout.addWidget(self.curve_view)

    def add_curve(self, x, y):
        line = pg.PlotDataItem(x, y)
        self.curve.addItem(line)



class MainWindow(QMainWindow):
    count = 0

    def __init__(self, parent=None):
        super(MainWindow, self).__init__(parent)
        self.mdi = QMdiArea()
        self.setCentralWidget(self.mdi)

        # Add menu
        bar = self.menuBar()
        file = bar.addMenu("File")
        # Add submenu
        file.addAction("New")
        file.addAction("cascade")
        file.addAction("Tiled")

        file.triggered[QAction].connect(self.windowaction)

        self.setWindowTitle("MDI demo")
        pg.setConfigOption("background", "w")

    def windowaction(self, q):
        if q.text() == "New":
            MainWindow.count = MainWindow.count + 1

            sub = QMdiSubWindow()

            pgwidget = NetworkPlot(self.mdi)
            pgwidget.add_curve(np.arange(10), np.random.random(10))

            sub.setWidget(pgwidget)

            sub.setWindowTitle("subWindow" + str(MainWindow.count))

            self.mdi.addSubWindow(sub)

            sub.show()
        if q.text() == "cascade":
            self.mdi.cascadeSubWindows()

        if q.text() == "Tiled":
            self.mdi.tileSubWindow()


if __name__ == "__main__":
    app = QApplication(sys.argv)
    demo = MainWindow()
    demo.show()
    sys.exit(app.exec_()

The screenshot of the plot result shows that the grid lines have inconsistent and irregular widths
20231106-101816

@pijyoi
Copy link
Contributor

pijyoi commented Nov 6, 2023

I have stripped down your example. The relevant point seems to be that you have chosen a non integer-sized pen width.
In addition, your example also chooses to modify the font and the alpha of the AxisItem, which may also be relevant.

import pyqtgraph as pg

pg.mkQApp()
pw = pg.PlotWidget()
pw.showGrid(True, True)

width = 1.75    # culprit
axis_pen = pg.mkPen(color="w", width=width)

for orientation in ("bottom", "left"):
    pw.getAxis(orientation).setPen(axis_pen)

pw.show()
pg.exec()

@j9ac9k
Copy link
Member

j9ac9k commented Nov 11, 2023

Wonder if we should have a helper method to check if we should use anti-aliasing or not, based on whether we have a non-integer pen thickness or if the lines are horizontal/vertical.

Also wonder if InfiniteLine line would suffer from the same issue...

@j9ac9k
Copy link
Member

j9ac9k commented Feb 17, 2024

Sorry for the long delay here. I moved cross country and had to pack up my computers and some of my laptops, so I have been unable to test this in a way that's needed. I need to investigate which platforms (macOS/windows/linux) and which combination of hidpi/non-hidpi screens as well. Anyway I'm getting setup where I'm temporarily residing so I should hopefully be able to start testing this more thoroughly in the near term.

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

4 participants