Skip to content

Commit

Permalink
Merge topic 'fix-line-scalar-coloring-macos'
Browse files Browse the repository at this point in the history
6246609 Fix cell coloring for thick lines on apple silicon

Acked-by: Kitware Robot <[email protected]>
Acked-by: buildbot <[email protected]>
Reviewed-by: Sankhesh Jhaveri <[email protected]>
Merge-request: !11794
  • Loading branch information
jspanchu authored and kwrobot committed Jan 10, 2025
2 parents 2e9e3d6 + 6246609 commit b97c978
Show file tree
Hide file tree
Showing 7 changed files with 163 additions and 0 deletions.
9 changes: 9 additions & 0 deletions Documentation/release/dev/fix-wide-lines-with-cell-colors.md
Original file line number Diff line number Diff line change
@@ -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.
1 change: 1 addition & 0 deletions Rendering/Core/Testing/Cxx/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,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
Expand Down
103 changes: 103 additions & 0 deletions Rendering/Core/Testing/Cxx/TestMixedGeometryCellScalars.cxx
Original file line number Diff line number Diff line change
@@ -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;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
2e1784ec9d9aba8f59119c7dcc115371a3343063bcd8fd964ff17c3cd08850cc8527948e6aaa9a03f660fe108bb392ecbad500db46d363a9ae859ed7adb59809
18 changes: 18 additions & 0 deletions Rendering/OpenGL2/vtkOpenGLPolyDataMapper.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

//------------------------------------------------------------------------------
Expand Down
20 changes: 20 additions & 0 deletions Rendering/OpenGL2/vtkOpenGLRenderWindow.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
#include "vtkTextureObjectVS.h" // a pass through shader

#include <cstdlib>
#include <cstring>
#include <sstream>
#include <string>
#include <type_traits>
Expand Down Expand Up @@ -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)
Expand Down
11 changes: 11 additions & 0 deletions Rendering/OpenGL2/vtkOpenGLRenderWindow.h
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,17 @@ class VTKRENDERINGOPENGL2_EXPORT VTK_MARSHALAUTO vtkOpenGLRenderWindow : 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
Expand Down

0 comments on commit b97c978

Please sign in to comment.