From 6246609959f03378164ad5a443250e757a5a4745 Mon Sep 17 00:00:00 2001 From: Jaswant Panchumarti <jaswant.panchumarti@kitware.com> Date: Mon, 30 Dec 2024 17:54:20 -0500 Subject: [PATCH] Fix cell coloring for thick lines on apple silicon - address paraview/paraview#22594 - address paraview/paraview#21832 - Thick lines were drawn by expanding a line to 2 triangles stacked on their longest side using a geometry shader. When coloring by cell scalars, this worked everywhere except on apple silicon. - There seems to be a bug related to geometry shaders in Apple's OpenGL driver that caused thick lines to not correctly pick up cell colors. - It looks like the value written to `gl_PrimitiveID` is overwritten implicitly by `EmitVertex` and incremented each time a vertex is emitted from the geometry shader. - This commit works around it by fixing the usage of `gl_PrimitiveID` in fragment shader when drawing thick lines. --- .../dev/fix-wide-lines-with-cell-colors.md | 9 ++ Rendering/Core/Testing/Cxx/CMakeLists.txt | 1 + .../Cxx/TestMixedGeometryCellScalars.cxx | 103 ++++++++++++++++++ .../TestMixedGeometryCellScalars.png.sha512 | 1 + Rendering/OpenGL2/vtkOpenGLPolyDataMapper.cxx | 18 +++ Rendering/OpenGL2/vtkOpenGLRenderWindow.cxx | 20 ++++ Rendering/OpenGL2/vtkOpenGLRenderWindow.h | 11 ++ 7 files changed, 163 insertions(+) create mode 100644 Documentation/release/dev/fix-wide-lines-with-cell-colors.md create mode 100644 Rendering/Core/Testing/Cxx/TestMixedGeometryCellScalars.cxx create mode 100644 Rendering/Core/Testing/Data/Baseline/TestMixedGeometryCellScalars.png.sha512 diff --git a/Documentation/release/dev/fix-wide-lines-with-cell-colors.md b/Documentation/release/dev/fix-wide-lines-with-cell-colors.md new file mode 100644 index 00000000000..a0c086847b9 --- /dev/null +++ b/Documentation/release/dev/fix-wide-lines-with-cell-colors.md @@ -0,0 +1,9 @@ +## Fix wide lines with cell colors for Apple silicon + +You can now render thick lines and map colors using cell scalars on Apple silicon. Previously, Apple silicon +chips would only render the first half of the line segments with cell scalar color and the rest of the lines +would not be drawn at all. VTK now handles this bug in upstream Apple driver by patching the shader code at +runtime. VTK applies the patch only for Apple OpenGL over Metal driver. + +You can check for the presence of this bug in Apple drivers with the +`bool vtkOpenGLRenderWindow::IsPrimIDBugPresent()` method. diff --git a/Rendering/Core/Testing/Cxx/CMakeLists.txt b/Rendering/Core/Testing/Cxx/CMakeLists.txt index 3d43a771ab9..9f1fb5dfad1 100644 --- a/Rendering/Core/Testing/Cxx/CMakeLists.txt +++ b/Rendering/Core/Testing/Cxx/CMakeLists.txt @@ -98,6 +98,7 @@ vtk_add_test_cxx(vtkRenderingCoreCxxTests tests TestManyActors.cxx,NO_VALID TestMapVectorsAsRGBColors.cxx TestMapVectorsToColors.cxx + TestMixedGeometryCellScalars.cxx,NO_DATA TestOffAxisStereo.cxx TestOnAndOffScreenConeCxx.cxx TestOpacity.cxx diff --git a/Rendering/Core/Testing/Cxx/TestMixedGeometryCellScalars.cxx b/Rendering/Core/Testing/Cxx/TestMixedGeometryCellScalars.cxx new file mode 100644 index 00000000000..d1eda3cd40e --- /dev/null +++ b/Rendering/Core/Testing/Cxx/TestMixedGeometryCellScalars.cxx @@ -0,0 +1,103 @@ +// SPDX-FileCopyrightText: Copyright (c) Ken Martin, Will Schroeder, Bill Lorensen +// SPDX-License-Identifier: BSD-3-Clause + +#include "vtkActor.h" +#include "vtkAppendPolyData.h" +#include "vtkArrayCalculator.h" +#include "vtkColorTransferFunction.h" +#include "vtkNew.h" +#include "vtkPolyDataMapper.h" +#include "vtkPolyLineSource.h" +#include "vtkProperty.h" +#include "vtkRegressionTestImage.h" +#include "vtkRegularPolygonSource.h" +#include "vtkRenderWindow.h" +#include "vtkRenderWindowInteractor.h" +#include "vtkRenderer.h" +#include "vtkSmartPointer.h" + +#include <initializer_list> +#include <string> + +namespace +{ +vtkSmartPointer<vtkAlgorithm> AddCellScalarArray( + const std::string expression, vtkSmartPointer<vtkAlgorithm> input) +{ + vtkNew<vtkArrayCalculator> calculator; + calculator->SetAttributeTypeToCellData(); + calculator->SetFunction(expression.c_str()); + calculator->SetInputConnection(input->GetOutputPort()); + return calculator; +} + +vtkSmartPointer<vtkAlgorithm> Append( + std::initializer_list<vtkSmartPointer<vtkAlgorithm>> inputAlgorithms) +{ + vtkNew<vtkAppendPolyData> append; + for (const auto& inputAlgorithm : inputAlgorithms) + { + append->AddInputConnection(inputAlgorithm->GetOutputPort()); + } + return append; +} +} + +int TestMixedGeometryCellScalars(int argc, char* argv[]) +{ + vtkNew<vtkPolyPointSource> points; + points->SetNumberOfPoints(5); + points->SetPoint(0, 0, 0, 0); + points->SetPoint(1, 1, 0, 0); + points->SetPoint(2, 2, 1, 0); + points->SetPoint(3, 2, 2, 0); + points->SetPoint(4, 1, 3, 0); + + vtkNew<vtkPolyLineSource> polyline; + polyline->SetClosed(false); + polyline->SetNumberOfPoints(5); + polyline->SetPoint(0, 0, 0, 0); + polyline->SetPoint(1, 1, 0, 0); + polyline->SetPoint(2, 2, 1, 0); + polyline->SetPoint(3, 2, 2, 0); + polyline->SetPoint(4, 1, 3, 0); + + vtkNew<vtkRegularPolygonSource> polygon; + polygon->SetGeneratePolyline(false); + polygon->SetCenter(5, 5, 0); + polygon->SetRadius(2); + polygon->SetNumberOfSides(8); + + auto append = ::Append({ ::AddCellScalarArray("0.1", points), + ::AddCellScalarArray("0.5", polyline), ::AddCellScalarArray("0.9", polygon) }); + + vtkNew<vtkPolyDataMapper> mapper; + mapper->SetInputConnection(append->GetOutputPort()); + + vtkNew<vtkColorTransferFunction> ctf; + ctf->AddRGBPoint(0.0, 1.0, 0.0, 0.0); + ctf->AddRGBPoint(0.5, 0.0, 0.5, 1.0); + ctf->AddRGBPoint(1.0, 0.0, 1.0, 0.0); + mapper->SetLookupTable(ctf); + + vtkNew<vtkActor> actor; + actor->SetMapper(mapper); + actor->GetProperty()->SetLineWidth(10); + actor->GetProperty()->SetPointSize(20); + + vtkNew<vtkRenderer> renderer; + renderer->AddActor(actor); + renderer->SetBackground(1, 1, 1); + + vtkNew<vtkRenderWindow> renderWindow; + renderWindow->AddRenderer(renderer); + + vtkNew<vtkRenderWindowInteractor> interactor; + interactor->SetRenderWindow(renderWindow); + int retVal = vtkRegressionTestImage(renderWindow); + if (retVal == vtkTesting::DO_INTERACTOR) + { + interactor->Start(); + } + return retVal == vtkTesting::FAILED ? EXIT_FAILURE : EXIT_SUCCESS; +} diff --git a/Rendering/Core/Testing/Data/Baseline/TestMixedGeometryCellScalars.png.sha512 b/Rendering/Core/Testing/Data/Baseline/TestMixedGeometryCellScalars.png.sha512 new file mode 100644 index 00000000000..ad3535c5e64 --- /dev/null +++ b/Rendering/Core/Testing/Data/Baseline/TestMixedGeometryCellScalars.png.sha512 @@ -0,0 +1 @@ +2e1784ec9d9aba8f59119c7dcc115371a3343063bcd8fd964ff17c3cd08850cc8527948e6aaa9a03f660fe108bb392ecbad500db46d363a9ae859ed7adb59809 diff --git a/Rendering/OpenGL2/vtkOpenGLPolyDataMapper.cxx b/Rendering/OpenGL2/vtkOpenGLPolyDataMapper.cxx index cc0c5de95e0..fab03fd7565 100644 --- a/Rendering/OpenGL2/vtkOpenGLPolyDataMapper.cxx +++ b/Rendering/OpenGL2/vtkOpenGLPolyDataMapper.cxx @@ -313,6 +313,24 @@ void vtkOpenGLPolyDataMapper::BuildShaders( shaders[i.first.ShaderType]->SetSource(ssrc); } } + + // Fix gl_PrimitiveID in fragment shader after all shader replacements. + // When oglRenderWindow->IsPrimIDBugPresent() returns true, the geometry shader + // ignores values written into gl_PrimitiveID and increments it per output primitive. + // So, here, undo the two increments per line segment with a divide-by-2. + std::string FSSource = shaders[vtkShader::Fragment]->GetSource(); + if (this->HaveWideLines(ren, actor)) + { + if (auto oglRenderWindow = vtkOpenGLRenderWindow::SafeDownCast(ren->GetRenderWindow())) + { + if (oglRenderWindow->IsPrimIDBugPresent()) + { + vtkShaderProgram::Substitute(FSSource, "gl_PrimitiveID", "gl_PrimitiveID / 2"); + } + } + } + + shaders[vtkShader::Fragment]->SetSource(FSSource); } //------------------------------------------------------------------------------ diff --git a/Rendering/OpenGL2/vtkOpenGLRenderWindow.cxx b/Rendering/OpenGL2/vtkOpenGLRenderWindow.cxx index 4d544d65dc1..9e7e971725c 100644 --- a/Rendering/OpenGL2/vtkOpenGLRenderWindow.cxx +++ b/Rendering/OpenGL2/vtkOpenGLRenderWindow.cxx @@ -57,6 +57,7 @@ #include "vtkTextureObjectVS.h" // a pass through shader #include <cstdlib> +#include <cstring> #include <sstream> #include <string> #include <type_traits> @@ -849,6 +850,25 @@ void vtkOpenGLRenderWindow::OpenGLInitState() this->SetAlphaBitPlanes(rgba[3]); } +//------------------------------------------------------------------------------ +bool vtkOpenGLRenderWindow::IsPrimIDBugPresent() +{ + if (this->Initialized) + { + const char* glVendor = reinterpret_cast<const char*>(glGetString(GL_VENDOR)); + const char* glVersion = reinterpret_cast<const char*>(glGetString(GL_VERSION)); + + if (!strcmp(glVendor, "Apple")) + { + if (strstr(glVersion, "Metal") != nullptr) + { + return true; + } + } + } + return false; +} + //------------------------------------------------------------------------------ int vtkOpenGLRenderWindow::GetDefaultTextureInternalFormat( int vtktype, int numComponents, bool needInt, bool needFloat, bool needSRGB) diff --git a/Rendering/OpenGL2/vtkOpenGLRenderWindow.h b/Rendering/OpenGL2/vtkOpenGLRenderWindow.h index 3155a22eabc..f7f54e7ffdc 100644 --- a/Rendering/OpenGL2/vtkOpenGLRenderWindow.h +++ b/Rendering/OpenGL2/vtkOpenGLRenderWindow.h @@ -247,6 +247,17 @@ public: */ virtual bool IsPointSpriteBugPresent() { return false; } + /** + * On `gl_PrimitiveID`, the spec says it is a counter that is incremented after every individual + * point, line or triangle primitive is processed. Almost all OpenGL implementations increment the + * counter per input primitive type. However, there seems to be a bug in the OpenGL over Metal + * used in Apple silicon that overwrites any value that the shader writes into `gl_PrimitiveID`. + * + * This method returns true if the OpenGL driver has a bug that causes geometry shaders + * to ignore writes to the gl_PrimitiveID. It checks the OpenGL vendor string for Apple and Metal. + */ + bool IsPrimIDBugPresent(); + /** * Get a mapping of vtk data types to native texture formats for this window * we put this on the RenderWindow so that every texture does not have to -- GitLab