Skip to content

Commit d89b882

Browse files
committed
GL: port Debug{Output,Group,Message} away from std::string.
Most of these are just inputs, so a compatibility StringStl.h include will do, the only exception is the callback for which there needs to stay a deprecated overload (which is internally delegated from the StringView one). Also explicitly testing with non-null-terminated strings -- the APIs take an explicit size so it shouldn't be a problem, but it's always good to have this verified independently. Drivers are crap, you know. One consequence of no longer using an impossible-to-forward-declare std::string is that I had to deinline the DebugGroup constructor because it no longer worked with just a forward-declared StringView.
1 parent e933849 commit d89b882

File tree

7 files changed

+229
-74
lines changed

7 files changed

+229
-74
lines changed

Diff for: doc/changelog.dox

+4
Original file line numberDiff line numberDiff line change
@@ -863,6 +863,10 @@ See also:
863863
particular, if `BUILD_DEPRECATED` is set but `MAGNUM_BUILD_DEPRECATED` not,
864864
the unprefixed options are also recognized. See also
865865
[mosra/corrade#139](https://github.com/mosra/corrade/issues/139).
866+
- @ref GL::DebugOutput::setCallback() taking a @ref std::string in the
867+
callback function pointer is deprecated in favor of a version taking a
868+
@ref Containers::StringView. See also [mosra/magnum#499](https://github.com/mosra/magnum/pull/499)
869+
and [mosra/magnum#596](https://github.com/mosra/magnum/pull/596).
866870
- The @cpp Array @ce, @cpp Array1D @ce, @cpp Array2D @ce and
867871
@cpp Array3D @ce types that were used exclusively for specifying
868872
@ref SamplerWrapping in various texture APIs are deprecated in favor of

Diff for: src/Magnum/GL/CMakeLists.txt

+2-1
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,8 @@ if(NOT MAGNUM_TARGET_WEBGL)
150150
list(APPEND MagnumGL_SRCS
151151
DebugOutput.cpp
152152

153-
Implementation/DebugState.cpp)
153+
Implementation/DebugState.cpp
154+
Implementation/defaultDebugCallback.h)
154155

155156
list(APPEND MagnumGL_HEADERS
156157
DebugOutput.h)

Diff for: src/Magnum/GL/DebugOutput.cpp

+41-15
Original file line numberDiff line numberDiff line change
@@ -26,20 +26,25 @@
2626
#include "DebugOutput.h"
2727

2828
#ifndef MAGNUM_TARGET_WEBGL
29+
#include <Corrade/Containers/StringView.h>
2930
#include <Corrade/Utility/Assert.h>
3031
#include <Corrade/Utility/Debug.h>
31-
#include <Corrade/Utility/DebugStl.h>
3232

3333
#include "Magnum/GL/Context.h"
3434
#include "Magnum/GL/Extensions.h"
3535
#include "Magnum/GL/Implementation/State.h"
3636
#include "Magnum/GL/Implementation/DebugState.h"
37+
#include "Magnum/GL/Implementation/defaultDebugCallback.h"
38+
39+
#ifdef MAGNUM_BUILD_DEPRECATED
40+
#include <string>
41+
#endif
3742

3843
namespace Magnum { namespace GL {
3944

4045
namespace Implementation {
4146

42-
void defaultDebugCallback(const DebugOutput::Source source, const DebugOutput::Type type, const UnsignedInt id, const DebugOutput::Severity severity, const std::string& string, std::ostream* out) {
47+
void defaultDebugCallback(const DebugOutput::Source source, const DebugOutput::Type type, const UnsignedInt id, const DebugOutput::Severity severity, const Containers::StringView string, std::ostream* out) {
4348
Debug output{out};
4449
output << "Debug output:";
4550

@@ -105,7 +110,7 @@ APIENTRY
105110
#endif
106111
callbackWrapper(GLenum source, GLenum type, GLuint id, GLenum severity, GLsizei length, const GLchar* message, const void* userParam) {
107112
const auto& callback = *static_cast<const Implementation::DebugState::MessageCallback*>(userParam);
108-
callback.callback(DebugOutput::Source(source), DebugOutput::Type(type), id, DebugOutput::Severity(severity), std::string{message, std::size_t(length)}, callback.userParam);
113+
callback.callback(DebugOutput::Source(source), DebugOutput::Type(type), id, DebugOutput::Severity(severity), {message, std::size_t(length)}, callback.userParam);
109114
}
110115

111116
}
@@ -149,8 +154,25 @@ void DebugOutput::setCallback(const Callback callback, const void* userParam) {
149154
Context::current().state().debug.callbackImplementation(callback);
150155
}
151156

157+
#ifdef MAGNUM_BUILD_DEPRECATED
158+
void DebugOutput::setCallback(void(*const callback)(Source, Type, UnsignedInt, Severity, const std::string&, const void*), const void* userParam) {
159+
/* This is a second delegation step after the callbackWrapper() which
160+
converts from raw GL types. Alternatively there could be a deprecated
161+
version of all callbackImplementation*() variants, but this is less
162+
code in total -- just two extra members in the MessageCallback
163+
struct. */
164+
Context::current().state().debug.messageCallback.userParam = &Context::current().state().debug.messageCallback;
165+
Context::current().state().debug.messageCallback.callbackStlString = callback;
166+
Context::current().state().debug.messageCallback.userParamStlString = userParam;
167+
Context::current().state().debug.callbackImplementation([](DebugOutput::Source source, DebugOutput::Type type, UnsignedInt id, DebugOutput::Severity severity, const Containers::StringView string, const void* userParam) {
168+
const auto& messageCallback = *static_cast<const Implementation::DebugState::MessageCallback*>(userParam);
169+
messageCallback.callbackStlString(source, type, id, severity, string, messageCallback.userParamStlString);
170+
});
171+
}
172+
#endif
173+
152174
void DebugOutput::setDefaultCallback() {
153-
setCallback([](DebugOutput::Source source, DebugOutput::Type type, UnsignedInt id, DebugOutput::Severity severity, const std::string& string, const void*) {
175+
setCallback([](DebugOutput::Source source, DebugOutput::Type type, UnsignedInt id, DebugOutput::Severity severity, const Containers::StringView string, const void*) {
154176
Implementation::defaultDebugCallback(source, type, id, severity, string, Debug::output());
155177
});
156178
}
@@ -267,30 +289,30 @@ Debug& operator<<(Debug& debug, const DebugOutput::Severity value) {
267289
}
268290
#endif
269291

270-
void DebugMessage::insertInternal(const Source source, const Type type, const UnsignedInt id, const DebugOutput::Severity severity, const Containers::ArrayView<const char> string) {
292+
void DebugMessage::insert(const Source source, const Type type, const UnsignedInt id, const DebugOutput::Severity severity, const Containers::StringView string) {
271293
Context::current().state().debug.messageInsertImplementation(source, type, id, severity, string);
272294
}
273295

274-
void DebugMessage::insertImplementationNoOp(Source, Type, UnsignedInt, DebugOutput::Severity, const Containers::ArrayView<const char>) {}
296+
void DebugMessage::insertImplementationNoOp(Source, Type, UnsignedInt, DebugOutput::Severity, const Containers::StringView) {}
275297

276298
#ifndef MAGNUM_TARGET_GLES2
277-
void DebugMessage::insertImplementationKhrDesktopES32(const Source source, const Type type, const UnsignedInt id, const DebugOutput::Severity severity, const Containers::ArrayView<const char> string) {
299+
void DebugMessage::insertImplementationKhrDesktopES32(const Source source, const Type type, const UnsignedInt id, const DebugOutput::Severity severity, const Containers::StringView string) {
278300
glDebugMessageInsert(GLenum(source), GLenum(type), id, GLenum(severity), string.size(), string.data());
279301
}
280302
#endif
281303

282304
#ifdef MAGNUM_TARGET_GLES
283-
void DebugMessage::insertImplementationKhrES(const Source source, const Type type, const UnsignedInt id, const DebugOutput::Severity severity, const Containers::ArrayView<const char> string) {
305+
void DebugMessage::insertImplementationKhrES(const Source source, const Type type, const UnsignedInt id, const DebugOutput::Severity severity, const Containers::StringView string) {
284306
glDebugMessageInsertKHR(GLenum(source), GLenum(type), id, GLenum(severity), string.size(), string.data());
285307
}
286308
#endif
287309

288-
void DebugMessage::insertImplementationExt(Source, Type, UnsignedInt, DebugOutput::Severity, const Containers::ArrayView<const char> string) {
310+
void DebugMessage::insertImplementationExt(Source, Type, UnsignedInt, DebugOutput::Severity, const Containers::StringView string) {
289311
glInsertEventMarkerEXT(string.size(), string.data());
290312
}
291313

292314
#ifndef MAGNUM_TARGET_GLES
293-
void DebugMessage::insertImplementationGremedy(Source, Type, UnsignedInt, DebugOutput::Severity, const Containers::ArrayView<const char> string) {
315+
void DebugMessage::insertImplementationGremedy(Source, Type, UnsignedInt, DebugOutput::Severity, const Containers::StringView string) {
294316
glStringMarkerGREMEDY(string.size(), string.data());
295317
}
296318
#endif
@@ -347,7 +369,11 @@ Int DebugGroup::maxStackDepth() {
347369
return value;
348370
}
349371

350-
void DebugGroup::pushInternal(const Source source, const UnsignedInt id, const Containers::ArrayView<const char> message) {
372+
DebugGroup::DebugGroup(const Source source, const UnsignedInt id, const Containers::StringView message): DebugGroup{} {
373+
push(source, id, message);
374+
}
375+
376+
void DebugGroup::push(const Source source, const UnsignedInt id, const Containers::StringView message) {
351377
CORRADE_ASSERT(!_active, "GL::DebugGroup::push(): group is already active", );
352378
Context::current().state().debug.pushGroupImplementation(source, id, message);
353379
_active = true;
@@ -359,21 +385,21 @@ void DebugGroup::pop() {
359385
_active = false;
360386
}
361387

362-
void DebugGroup::pushImplementationNoOp(Source, UnsignedInt, Containers::ArrayView<const char>) {}
388+
void DebugGroup::pushImplementationNoOp(Source, UnsignedInt, Containers::StringView) {}
363389

364390
#ifndef MAGNUM_TARGET_GLES2
365-
void DebugGroup::pushImplementationKhrDesktopES32(const Source source, const UnsignedInt id, const Containers::ArrayView<const char> message) {
391+
void DebugGroup::pushImplementationKhrDesktopES32(const Source source, const UnsignedInt id, const Containers::StringView message) {
366392
glPushDebugGroup(GLenum(source), id, message.size(), message.data());
367393
}
368394
#endif
369395

370396
#ifdef MAGNUM_TARGET_GLES
371-
void DebugGroup::pushImplementationKhrES(const Source source, const UnsignedInt id, const Containers::ArrayView<const char> message) {
397+
void DebugGroup::pushImplementationKhrES(const Source source, const UnsignedInt id, const Containers::StringView message) {
372398
glPushDebugGroupKHR(GLenum(source), id, message.size(), message.data());
373399
}
374400
#endif
375401

376-
void DebugGroup::pushImplementationExt(Source, UnsignedInt, const Containers::ArrayView<const char> message) {
402+
void DebugGroup::pushImplementationExt(Source, UnsignedInt, const Containers::StringView message) {
377403
glPushGroupMarkerEXT(message.size(), message.data());
378404
}
379405

Diff for: src/Magnum/GL/DebugOutput.h

+38-44
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,20 @@
3131
*/
3232
#endif
3333

34-
#include <string>
35-
#include <Corrade/Containers/ArrayView.h>
34+
#include <initializer_list>
35+
#include <Corrade/Containers/Containers.h>
3636

3737
#include "Magnum/Magnum.h"
3838
#include "Magnum/GL/OpenGL.h"
3939
#include "Magnum/GL/visibility.h"
4040

41+
#ifdef MAGNUM_BUILD_DEPRECATED
42+
/* For all stuff that took std::string before */
43+
#include <Corrade/Containers/StringStl.h>
44+
/* For the deprecated std::string callback */
45+
#include <Corrade/Utility/StlForwardString.h>
46+
#endif
47+
4148
#ifndef MAGNUM_TARGET_WEBGL
4249
namespace Magnum { namespace GL {
4350

@@ -287,7 +294,7 @@ class MAGNUM_GL_EXPORT DebugOutput {
287294
*
288295
* @see @ref setCallback()
289296
*/
290-
typedef void(*Callback)(Source, Type, UnsignedInt, Severity, const std::string&, const void*);
297+
typedef void(*Callback)(Source, Type, UnsignedInt, Severity, Containers::StringView, const void*);
291298

292299
/**
293300
* @brief Max count of debug messages in log
@@ -394,6 +401,22 @@ class MAGNUM_GL_EXPORT DebugOutput {
394401
*/
395402
static void setCallback(Callback callback, const void* userParam = nullptr);
396403

404+
#ifdef MAGNUM_BUILD_DEPRECATED
405+
/**
406+
* @brief Set debug message callback
407+
* @m_deprecated_since_latest Use a @ref Callback taking a
408+
* @ref Containers::StringView instead.
409+
*/
410+
CORRADE_DEPRECATED("use a callback taking a Containers::StringView instead") static void setCallback(void(*callback)(Source, Type, UnsignedInt, Severity, const std::string&, const void*), const void* userParam = nullptr);
411+
#ifndef DOXYGEN_GENERATING_OUTPUT
412+
/* Because otherwise the call is ambiguous with nullptr due to the
413+
deprecated overload */
414+
static void setCallback(std::nullptr_t, const void* userParam = nullptr) {
415+
setCallback(static_cast<Callback>(nullptr), userParam);
416+
}
417+
#endif
418+
#endif
419+
397420
/**
398421
* @brief Set default debug message callback
399422
*
@@ -587,30 +610,22 @@ class MAGNUM_GL_EXPORT DebugMessage {
587610
* @fn_gl_extension_keyword{InsertEventMarker,EXT,debug_marker} or
588611
* @fn_gl_extension_keyword{StringMarker,GREMEDY,string_marker}
589612
*/
590-
static void insert(Source source, Type type, UnsignedInt id, DebugOutput::Severity severity, const std::string& string) {
591-
insertInternal(source, type, id, severity, {string.data(), string.size()});
592-
}
593-
594-
/** @overload */
595-
template<std::size_t size> static void insert(Source source, Type type, UnsignedInt id, DebugOutput::Severity severity, const char(&string)[size]) {
596-
insertInternal(source, type, id, severity, {string, size - 1});
597-
}
613+
static void insert(Source source, Type type, UnsignedInt id, DebugOutput::Severity severity, Containers::StringView string);
598614

599615
/** @brief There's no point in having an instance of this class */
600616
DebugMessage() = delete;
601617

602618
private:
603-
static void insertInternal(Source source, Type type, UnsignedInt id, DebugOutput::Severity severity, Containers::ArrayView<const char> string);
604-
static MAGNUM_GL_LOCAL void insertImplementationNoOp(Source, Type, UnsignedInt, DebugOutput::Severity, Containers::ArrayView<const char>);
619+
static MAGNUM_GL_LOCAL void insertImplementationNoOp(Source, Type, UnsignedInt, DebugOutput::Severity, Containers::StringView);
605620
#ifndef MAGNUM_TARGET_GLES2
606-
static MAGNUM_GL_LOCAL void insertImplementationKhrDesktopES32(Source source, Type type, UnsignedInt id, DebugOutput::Severity severity, Containers::ArrayView<const char> string);
621+
static MAGNUM_GL_LOCAL void insertImplementationKhrDesktopES32(Source source, Type type, UnsignedInt id, DebugOutput::Severity severity, Containers::StringView string);
607622
#endif
608623
#ifdef MAGNUM_TARGET_GLES
609-
static MAGNUM_GL_LOCAL void insertImplementationKhrES(Source source, Type type, UnsignedInt id, DebugOutput::Severity severity, Containers::ArrayView<const char> string);
624+
static MAGNUM_GL_LOCAL void insertImplementationKhrES(Source source, Type type, UnsignedInt id, DebugOutput::Severity severity, Containers::StringView string);
610625
#endif
611-
static MAGNUM_GL_LOCAL void insertImplementationExt(Source, Type, UnsignedInt, DebugOutput::Severity, Containers::ArrayView<const char> string);
626+
static MAGNUM_GL_LOCAL void insertImplementationExt(Source, Type, UnsignedInt, DebugOutput::Severity, Containers::StringView string);
612627
#ifndef MAGNUM_TARGET_GLES
613-
static MAGNUM_GL_LOCAL void insertImplementationGremedy(Source, Type, UnsignedInt, DebugOutput::Severity, Containers::ArrayView<const char> string);
628+
static MAGNUM_GL_LOCAL void insertImplementationGremedy(Source, Type, UnsignedInt, DebugOutput::Severity, Containers::StringView string);
614629
#endif
615630
};
616631

@@ -734,14 +749,7 @@ class MAGNUM_GL_EXPORT DebugGroup {
734749
* Calls @ref push().
735750
* @see @link ~DebugGroup() @endlink, @ref pop()
736751
*/
737-
explicit DebugGroup(Source source, UnsignedInt id, const std::string& message): DebugGroup{} {
738-
push(source, id, message);
739-
}
740-
741-
/** @overload */
742-
template<std::size_t size> explicit DebugGroup(Source source, UnsignedInt id, const char(&message)[size]): DebugGroup{} {
743-
push(source, id, message);
744-
}
752+
explicit DebugGroup(Source source, UnsignedInt id, Containers::StringView message);
745753

746754
/**
747755
* @brief Destructor
@@ -768,14 +776,7 @@ class MAGNUM_GL_EXPORT DebugGroup {
768776
* @ref Renderer::Error::StackOverflow, @fn_gl_keyword{PushDebugGroup}
769777
* or @fn_gl_extension_keyword{PushGroupMarker,EXT,debug_marker}
770778
*/
771-
void push(Source source, UnsignedInt id, const std::string& message) {
772-
pushInternal(source, id, {message.data(), message.size()});
773-
}
774-
775-
/** @overload */
776-
template<std::size_t size> void push(Source source, UnsignedInt id, const char(&message)[size]) {
777-
pushInternal(source, id, {message, size - 1});
778-
}
779+
void push(Source source, UnsignedInt id, Containers::StringView message);
779780

780781
/**
781782
* @brief Pop debug group from the stack
@@ -798,16 +799,14 @@ class MAGNUM_GL_EXPORT DebugGroup {
798799
void pop();
799800

800801
private:
801-
void pushInternal(Source source, UnsignedInt id, Containers::ArrayView<const char> message);
802-
803-
static MAGNUM_GL_LOCAL void pushImplementationNoOp(Source source, UnsignedInt id, Containers::ArrayView<const char> message);
802+
static MAGNUM_GL_LOCAL void pushImplementationNoOp(Source source, UnsignedInt id, Containers::StringView message);
804803
#ifndef MAGNUM_TARGET_GLES2
805-
static MAGNUM_GL_LOCAL void pushImplementationKhrDesktopES32(Source source, UnsignedInt id, Containers::ArrayView<const char> message);
804+
static MAGNUM_GL_LOCAL void pushImplementationKhrDesktopES32(Source source, UnsignedInt id, Containers::StringView message);
806805
#endif
807806
#ifdef MAGNUM_TARGET_GLES
808-
static MAGNUM_GL_LOCAL void pushImplementationKhrES(Source source, UnsignedInt id, Containers::ArrayView<const char> message);
807+
static MAGNUM_GL_LOCAL void pushImplementationKhrES(Source source, UnsignedInt id, Containers::StringView message);
809808
#endif
810-
static MAGNUM_GL_LOCAL void pushImplementationExt(Source source, UnsignedInt id, Containers::ArrayView<const char> message);
809+
static MAGNUM_GL_LOCAL void pushImplementationExt(Source source, UnsignedInt id, Containers::StringView message);
811810

812811
static MAGNUM_GL_LOCAL void popImplementationNoOp();
813812
#ifndef MAGNUM_TARGET_GLES2
@@ -824,11 +823,6 @@ class MAGNUM_GL_EXPORT DebugGroup {
824823
/** @debugoperatorclassenum{DebugGroup,DebugGroup::Source} */
825824
MAGNUM_GL_EXPORT Debug& operator<<(Debug& debug, DebugGroup::Source value);
826825

827-
/* Exposed for testing */
828-
namespace Implementation {
829-
MAGNUM_GL_EXPORT void defaultDebugCallback(DebugOutput::Source source, DebugOutput::Type type, UnsignedInt id, DebugOutput::Severity severity, const std::string& string, std::ostream* output);
830-
}
831-
832826
}}
833827
#else
834828
#error this header is not available in WebGL build

Diff for: src/Magnum/GL/Implementation/DebugState.h

+6-2
Original file line numberDiff line numberDiff line change
@@ -40,16 +40,20 @@ struct DebugState {
4040
Containers::String(*getLabelImplementation)(GLenum, GLuint);
4141
void(*labelImplementation)(GLenum, GLuint, Containers::StringView);
4242

43-
void(*messageInsertImplementation)(DebugMessage::Source, DebugMessage::Type, UnsignedInt, DebugOutput::Severity, Containers::ArrayView<const char>);
43+
void(*messageInsertImplementation)(DebugMessage::Source, DebugMessage::Type, UnsignedInt, DebugOutput::Severity, Containers::StringView);
4444
void(*controlImplementation)(GLenum, GLenum, GLenum, std::initializer_list<UnsignedInt>, bool);
4545
void(*callbackImplementation)(DebugOutput::Callback);
46-
void(*pushGroupImplementation)(DebugGroup::Source, UnsignedInt, Containers::ArrayView<const char>);
46+
void(*pushGroupImplementation)(DebugGroup::Source, UnsignedInt, Containers::StringView);
4747
void(*popGroupImplementation)();
4848

4949
GLint maxLabelLength, maxLoggedMessages, maxMessageLength, maxStackDepth;
5050
struct MessageCallback {
5151
DebugOutput::Callback callback{};
5252
const void* userParam{};
53+
#ifdef MAGNUM_BUILD_DEPRECATED
54+
void(*callbackStlString)(DebugOutput::Source, DebugOutput::Type, UnsignedInt, DebugOutput::Severity, const std::string&, const void*);
55+
const void* userParamStlString{};
56+
#endif
5357
} messageCallback;
5458
};
5559

0 commit comments

Comments
 (0)