Adding the capture options to annotation for display with info tool #1265
Conversation
|
CI gfxreconstruct build queued with queue ID 46068. |
|
CI gfxreconstruct build # 3219 running. |
|
CI gfxreconstruct build # 3219 failed. |
|
This change relies on #1155 to be merged first. |
bradgrantham-lunarg
added
the
P2
Several options are recommended when dealing with visual corruption on traces. It would be good to know, which options were used when capturing a trace. This change captures this information, if non-default parameters were used. Change-Id: I9c09198ed08649a037d6d789aaef93c75beb5848
naomipappe
force-pushed
the
feature-capture-parameters
branch
from
ee402d0 to
7f3ca93
Compare
|
CI gfxreconstruct build queued with queue ID 94664. |
|
CI gfxreconstruct build # 3588 running. |
|
CI gfxreconstruct build # 3588 failed. |
|
The LunarG CI failure appears to be due to an unrelated GPU crash. I've restored the machine and will restart the test. |
|
CI gfxreconstruct build queued with queue ID 94959. |
|
CI gfxreconstruct build # 3589 running. |
|
CI gfxreconstruct build # 3589 passed. |
tools/info/main.cpp
Outdated
| try | ||
| { | ||
| out = json_obj.at(key).get<std::string>(); | ||
| } | ||
| catch (const nlohmann::json::type_error& te) | ||
| { | ||
| out += "\n\t" + json_obj.at(key).dump(kDefaultIndent); | ||
| out.pop_back(); | ||
| out += "\t}"; | ||
| } |
There was a problem hiding this comment.
The use of try-catch here feels awkward, as it should be possible to know if its a string or not ahead of time.
Should be possible to call is_string() before printing, and using the fallback if it isn't a string.
https://json.nlohmann.me/api/basic_json/is_string/
I say this because other uses of try-catch in the codebase are in main() functions, or are handling potentially invalid input (like stof), whereas here there is a way to "know" what to do without relying on an exception to do so.
There was a problem hiding this comment.
Oh yes, somehow I missed this method while I was reading the documentation. Your suggestion makes much more sense. Furthermore, thanks to your comment I revisited this piece of code, and made further adjustments.
Change-Id: I1d51d88093f4259e3d4b962e1abf0ef01802a271
|
CI gfxreconstruct build queued with queue ID 95438. |
|
CI gfxreconstruct build # 3597 running. |
|
CI gfxreconstruct build # 3597 passed. |
|
I think it's probably best to remove any functionality from this PR that depends on #1155 and submit those dependencies as a separate PR - it sounds like @charles-lunarg has approved this and this can go in quickly. |
|
The dependency that it had on #1155 was the display of the fence delay option that PR added. I have removed that when I updated the PR recently. |
Several options are recommended when dealing with visual corruption on traces. It would be good to know, which non-default options were used when capturing a trace.
This change captures this information into the operation annotation, if non-default parameters were used, and can be viewed with
gfxrecon-infotool.