Duplicate GetGroup/GetBinding in vtkWebGPUComputeBuffer.h

Development
1

Building VTK 9.6.2 with WebGPU enabled on Windows / MSVC fails to compile Rendering/WebGPU/vtkWebGPUComputeBuffer.h.

Group and Binding are declared as vtkIdType members but each accessor is defined twice, once hand-written returning uint32_t, and once via vtkGetMacro returning vtkIdType:

// lines 91-92
uint32_t GetGroup() const { return this->Group; }
vtkGetMacro(Group, vtkIdType);     // expands to vtkIdType GetGroup() const

// lines 104-105
uint32_t GetBinding() const { return this->Binding; }
vtkGetMacro(Binding, vtkIdType);   // expands to vtkIdType GetBinding() const

After macro expansion each is a redefinition of GetGroup() / GetBinding() differing only by return type (uint32_t vs vtkIdType), which is ill-formed:

vtkWebGPUComputeBuffer.h(92): error C2556:
'vtkIdType vtkWebGPUComputeBuffer::GetGroup(void) const': overloaded function differs only by return type from 'uint32_t vtkWebGPUComputeBuffer::GetGroup(void) const'
vtkWebGPUComputeBuffer.h(92): error C2371: 'vtkWebGPUComputeBuffer::GetGroup': redefinition; different basic types

Are we doing something wrong? Cause that must have gone through CI/CD at some point.

Thanks!

2

We discovered that VTK_USE_FUTURE_CONST=ON triggers it. Without it, vtkGetMacro expands to a non-const getter which doesn’t interfere with the const ones. We can disable it for now but this is a potential bug.

3

@jaswantp It looks like @tclabault wrote this originally, but we should investigate it. A method changing return type based on const is confusing anyways.

4

we should remove the overload that returns uint32_t.

5

Please note that this happens in other WebGPU headers too with the same problem. I forgot which ones but there were at least two others.

6

I dont think VTK_USE_FUTURE_CONST=ON is reader for prime time yet, it is only tested independantly by @seanm , but maybe will be turned on in some CI in the coming months.

7

Thanks @mwestphal , sounds good.

That said, VTK_USE_FUTURE_BOOL could use the same treatment as some vtkSetClampMacro which are now bool are trying to force int clamping values resulting in a compile time warning (or error if treated as such).

8

should the uint32_t getters be deprecated? or removed?

note that deprecating them will not address your issue unless you set VTK_LEGACY_REMOVE which may have side effects.

9

Well, I only use a subset of VTK of course, but we’ve been shipping our app built with VTK_USE_FUTURE_CONST for years. It’s ready for prime time in that sense.

10

Even if it isn’t ready, this is a bug regardless of the setting. It’s just that the setting actually uncovered an issue in this case.