Skip to content

Conversation

@xinitrcn1
Copy link

Description

Tested myself, works on all these systems.
Obviously RenderDoc itself doesn't even work on FreeBSD. But the API does on all these unixes!

@baldurk
Copy link
Owner

baldurk commented Oct 21, 2025

If RenderDoc doesn't work then how did you test this? And what is the purpose of this change?

@xinitrcn1
Copy link
Author

If RenderDoc doesn't work then how did you test this? And what is the purpose of this change?

this is mainly because some applications straight up vendor renderdoc_app.h and do not change anything for portability
while yes - one could patch them manually, i tought it would be easier to just have that be in upstream

@baldurk
Copy link
Owner

baldurk commented Oct 21, 2025

I'm not sure that this is a good idea. If RenderDoc does not work then this feels like a hack in the wrong place - if an application wants to compile on another OS that RenderDoc doesn't support then they should conditionally include the header so that then they can also conditionally use the functionality.

The same would apply to any other header or library with limited platform support - if there were a library that e.g. did not support Android then it's up to an application to only reference it on desktop.

I think I'd be OK with demoting the #error to a #warning and adding an #ifndef RENDERDOC_CC around the whole of the header so that it becomes a no-op to include on unsupported platforms.

@xinitrcn1
Copy link
Author

xinitrcn1 commented Oct 21, 2025

I'm not sure that this is a good idea. If RenderDoc does not work then this feels like a hack in the wrong place - if an application wants to compile on another OS that RenderDoc doesn't support then they should conditionally include the header so that then they can also conditionally use the functionality.

The same would apply to any other header or library with limited platform support - if there were a library that e.g. did not support Android then it's up to an application to only reference it on desktop.

I think I'd be OK with demoting the #error to a #warning and adding an #ifndef RENDERDOC_CC around the whole of the header so that it becomes a no-op to include on unsupported platforms.

What about demotion + #define RENDERDOC_CC. If in the future RenderDoc is ported - then an application would not need to be recompiled (except maybe on Atari OS or AROS)

@baldurk
Copy link
Owner

baldurk commented Oct 21, 2025

No the header should not expose functionality that's untested and non-functional. As I say really applications should not be including headers that don't work.

@xinitrcn1
Copy link
Author

It's not just applications. Mesa itself does this
https://gitlab.freedesktop.org/mesa/mesa/-/blob/25.3/include/renderdoc_app.h?ref_type=heads#L43

Unless there is a way to do things without downloads (or CMake hell) applications will continue to vendor RenderDoc for the foreseeable future. Maybe forbidding such vendoring in the first place (like making renderdoc_app.h depend on another header or whatnot) would be ideal to stop the accidentally stepping on platforms without renderdoc.

Or all of this is solved with a simple #ifdef on all the applications; but again I went to the source upstream to at-least try to fix the breakage in one go.

@baldurk
Copy link
Owner

baldurk commented Oct 22, 2025

I'm not sure how vendoring is related here. Copying the header into the tree and using it from there is a good idea and keeps things simple, but that's entirely independent from whether or not it gets included.

An example from mesa would be the D3D12 headers - they are used in some places but are not guaranteed to compile on all platforms especially when they're only meaningful on windows. How they're distributed is immaterial, they should still only be included on windows.

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.

2 participants