From 7ea354d01164f97621cee77bc87fb341cf16b0e2 Mon Sep 17 00:00:00 2001 From: Lawrence L Love Date: Thu, 24 Apr 2014 12:16:49 -0700 Subject: [PATCH 1/2] vogleditor: add nested operations check A check is added at the "glEnd" entrypoint to ensure it doesn't get moved past the parent frame in the case of an unpaired glEnd(). This can inadvertently occur in a program and will cause a crash. (Similar defect in apitrace) Signed-off-by: Lawrence L Love --- src/vogleditor/vogleditor_qapicalltreemodel.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/vogleditor/vogleditor_qapicalltreemodel.cpp b/src/vogleditor/vogleditor_qapicalltreemodel.cpp index 107843c7..78e306d9 100644 --- a/src/vogleditor/vogleditor_qapicalltreemodel.cpp +++ b/src/vogleditor/vogleditor_qapicalltreemodel.cpp @@ -247,15 +247,17 @@ void vogleditor_QApiCallTreeModel::setupModelData(vogl_trace_file_reader* pTrace else if (entrypoint_id == VOGL_ENTRYPOINT_glEnd) { // move the parent back one level of the hierarchy, to its own parent - pCurParent = pCurParent->parent(); + // (but not past Frame parent [e.g., unpaired "glEnd" operation]) + if (pCurParent->parent() != parent) + pCurParent = pCurParent->parent(); } } if (pTrace_reader->get_packet_type() == cTSPTEOF) { - //found_eof_packet = true; - vogl_printf("Found trace file EOF packet on swap %" PRIu64 "\n", total_swaps); - break; + //found_eof_packet = true; + vogl_printf("Found trace file EOF packet on swap %" PRIu64 "\n", total_swaps); + break; } } } From 817685d7481531de15dedebb694a0ce5c2a07e93 Mon Sep 17 00:00:00 2001 From: Lawrence L Love Date: Thu, 24 Apr 2014 12:35:26 -0700 Subject: [PATCH 2/2] vogleditor: Add for additional nest operations Add glPush/PopDebugGroup as nesting delimiters along with existing glBegin/End. Use same method as for other "vogl_is_xxx_entrypoint" operations. Comments: If this glPush/PopDebugGroup addition is (and/or possibly others are) accepted for nesting, another method would be to add a flag attribute to packets to identify apicall types that can be logically AND'd rather than using a switch statement (as done in apitrace). Alternatively, create a "nest" object to encapsulate this information, initializing it with a set of nested pairs (statically or via some preference) and provide an interface(s) to return needed information. Signed-off-by: Lawrence L Love --- src/voglcommon/vogl_gl_utils.cpp | 30 +++++++++++++++++++ src/voglcommon/vogl_gl_utils.h | 2 ++ .../vogleditor_qapicalltreemodel.cpp | 9 +++--- 3 files changed, 37 insertions(+), 4 deletions(-) diff --git a/src/voglcommon/vogl_gl_utils.cpp b/src/voglcommon/vogl_gl_utils.cpp index 2651e2f5..15bce3e5 100644 --- a/src/voglcommon/vogl_gl_utils.cpp +++ b/src/voglcommon/vogl_gl_utils.cpp @@ -1344,6 +1344,36 @@ bool vogl_is_clear_entrypoint(gl_entrypoint_id_t id) return false; } +//------------------------------------------------------------------------------ +// vogl_is_start_nested_entrypoint +//------------------------------------------------------------------------------ +bool vogl_is_start_nested_entrypoint(gl_entrypoint_id_t id) +{ + switch (id) { + case VOGL_ENTRYPOINT_glBegin: + case VOGL_ENTRYPOINT_glPushDebugGroup: + return true; + default: + break; + } + return false; +} + +//------------------------------------------------------------------------------ +// vogl_is_end_nested_entrypoint +//------------------------------------------------------------------------------ +bool vogl_is_end_nested_entrypoint(gl_entrypoint_id_t id) +{ + switch (id) { + case VOGL_ENTRYPOINT_glEnd: + case VOGL_ENTRYPOINT_glPopDebugGroup: + return true; + default: + break; + } + return false; +} + //---------------------------------------------------------------------------------------------------------------------- // vogl_get_json_value_as_enum //---------------------------------------------------------------------------------------------------------------------- diff --git a/src/voglcommon/vogl_gl_utils.h b/src/voglcommon/vogl_gl_utils.h index f6841108..c86e8eca 100644 --- a/src/voglcommon/vogl_gl_utils.h +++ b/src/voglcommon/vogl_gl_utils.h @@ -379,6 +379,8 @@ inline bool vogl_is_swap_buffers_entrypoint(gl_entrypoint_id_t id) bool vogl_is_draw_entrypoint(gl_entrypoint_id_t id); bool vogl_is_clear_entrypoint(gl_entrypoint_id_t id); +bool vogl_is_start_nested_entrypoint(gl_entrypoint_id_t id); +bool vogl_is_end_nested_entrypoint(gl_entrypoint_id_t id); //---------------------------------------------------------------------------------------------------------------------- // Error helpers diff --git a/src/vogleditor/vogleditor_qapicalltreemodel.cpp b/src/vogleditor/vogleditor_qapicalltreemodel.cpp index 78e306d9..a8adbf8d 100644 --- a/src/vogleditor/vogleditor_qapicalltreemodel.cpp +++ b/src/vogleditor/vogleditor_qapicalltreemodel.cpp @@ -239,15 +239,16 @@ void vogleditor_QApiCallTreeModel::setupModelData(vogl_trace_file_reader* pTrace // reset the CurFrame so that a new frame node will be created on the next api call pCurFrame = NULL; } - else if (entrypoint_id == VOGL_ENTRYPOINT_glBegin) + else if (vogl_is_start_nested_entrypoint(entrypoint_id)) { - // items in the glBegin/glEnd block will be nested, including the glEnd + // Nest logically paired blocks of gl calls including terminating + // nest call pCurParent = item; } - else if (entrypoint_id == VOGL_ENTRYPOINT_glEnd) + else if (vogl_is_end_nested_entrypoint(entrypoint_id)) { // move the parent back one level of the hierarchy, to its own parent - // (but not past Frame parent [e.g., unpaired "glEnd" operation]) + // (but not past Frame parent [e.g., unpaired "end" operation]) if (pCurParent->parent() != parent) pCurParent = pCurParent->parent(); }