-
-
Notifications
You must be signed in to change notification settings - Fork 19.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Per object rid info for 3.2 #43830
Per object rid info for 3.2 #43830
Conversation
f2ff701
to
cb03acb
Compare
return new_info; | ||
} | ||
|
||
bool has_shadows() { return true; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Delete has shadows.
case RENDER_2D_ITEMS_IN_FRAME: return VS::get_singleton()->get_render_info(VS::INFO_2D_ITEMS_IN_FRAME); | ||
case RENDER_2D_DRAW_CALLS_IN_FRAME: return VS::get_singleton()->get_render_info(VS::INFO_2D_DRAW_CALLS_IN_FRAME); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing 2d items.
vp->render_info[VS::VIEWPORT_RENDER_INFO_2D_ITEMS_IN_FRAME] = VSG::storage->get_captured_render_info(VS::INFO_2D_ITEMS_IN_FRAME); | ||
vp->render_info[VS::VIEWPORT_RENDER_INFO_2D_DRAW_CALLS_IN_FRAME] = VSG::storage->get_captured_render_info(VS::INFO_2D_DRAW_CALLS_IN_FRAME); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing 2d items.
} | ||
render.vertices_count += s->array_len * amount; | ||
storage->info.rid_render_info_render[s->mesh] = render; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this kind of thing it looks like it may be better just to store the counts in a temporary variable without an if,
int stats_vertex_count = 0;
stats_vertex_count += s->array_len * amount;
then have one if statement at the end of the function ..
if (storing_debug_stuff)
...
Just incrementing the counts isn't going to be much of a hit for 3d, even when stats are not being used, in fact the if statements may be more expensive than just doing the counts...
This way the stats would be less intrusive and easier to maintain.
info_write[VS::INFO_SURFACE_CHANGES_IN_FRAME] = info.snap.surface_switch_count; | ||
info_write[VS::INFO_DRAW_CALLS_IN_FRAME] = info.snap.draw_call_count; | ||
return new_info; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't this simply be passed as a structure to fill in?
e.g.
get_captured_render_info(StatsRenderInfo &r_info)
{
r_info.objects_in_frame = BLAH;
...
}
or directly return the structure as a const ref?
Ah maybe this is for some thread protection or something. Still seems like redundant copying is going on.
I'm not actually so familiar with the 3d rasterizer, but don't we maintain stats already for the overall number of calls..., yes Perhaps an alternative paradigm could be to just note when we start on an object which is selected for stats, record the |
Overall though more render stats are welcome 👍 imo, I did a proposal on this too: Myself I'm also particularly interested in being able to display non rendered stats such as the number of source verts (rather than the number of indices). I should edit that proposal to make that clear actually. The number of source verts gives an indication of the success of an import and whether vertices are being shared or too many are unique. This would not need to go via the rasterizer, it could be a query of the mesh instance itself. You say you don't expect to be merged, but providing it can be made streamlined and as least intrusive as possible speaking for myself I could see this as beneficial in 3.2. The most important factor in my opinion is not having the stats complicate reading the rendering (in terms of verbosity or where the code is), or serve as a barrier to rearranging the rendering. But @clayjohn will chime in too I'm sure and we can ask reduz if you decide to go for it. Also just to be clear as I haven't compiled this and tested, can this be used in game exports? |
Since this PR is for 3.2 and I don't have the effort to pr this for master, I'm going to close the PR. It is salvageable. |
Made a pr branch for @lawnjelly to review per rid object info. Per rid object info is not trivial to port to Godot Engine master, but I wanted to share.
Partially supports godotengine/godot-proposals#63 by having individual object information.
Improvement notes:
I don't expect this to be merged in 3.2.
Sponsored by IMVU.