-
-
Notifications
You must be signed in to change notification settings - Fork 277
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
Render context lost handling pt.1 #8900
base: dev
Are you sure you want to change the base?
Conversation
ba9c476
to
c54cc25
Compare
Implement rendering pause. Add missing implementation. Add render resume message.
c54cc25
to
13fd2a7
Compare
message RenderContextLost | ||
{ | ||
} |
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.
Is this only for game objects?
I kind of would have expected render_ddf.proio
to own a "render context lost" message.
bool InvalidateIterator(const IteratorResource& resource, void* user_ctx) | ||
{ | ||
HFactory factory = (HFactory)user_ctx; | ||
InvalidateGraphicsHandle(factory, resource.m_Resource); | ||
return true; | ||
} | ||
|
||
void InvalidateGraphicsResources(HFactory factory) | ||
{ | ||
IterateResources(factory, &InvalidateIterator, factory); | ||
} |
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.
Are these only used in this file?
Then we prefer to add "static" to the functions, to make them private. (for readability and possible compiler/linker optimizations)
void InvalidatePropertyResources(dmResource::HFactory factory, dmArray<void*>& resources) | ||
{ | ||
for (uint32_t i = 0; i < resources.Size(); ++i) | ||
{ | ||
dmResource::InvalidateGraphicsHandle(factory, resources[i]); | ||
} | ||
} |
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.
This feels a little bit too granular.
Wouldn't it work the resource system to just go through all resources in a single for-loop?
TileGridWorld* world = (TileGridWorld*) params.m_World; | ||
if (world->m_VertexDeclaration) | ||
{ | ||
// dmGraphics::DeleteVertexDeclaration(world->m_VertexDeclaration); |
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.
dead code
if (world->m_VertexDeclaration) | ||
{ | ||
// dmGraphics::DeleteVertexDeclaration(world->m_VertexDeclaration); | ||
dmRender::InvalidateBufferedRenderBuffer(world->m_VertexBuffer); |
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.
The index buffer needs invalidation too?
And, this is my biggest concern with this change, and it's that it requires a lot of detailed fixes.
But, just as dmResource can invalidate all graphics resources, perhaps dmRender could too?
I haven't discussed it with @Jhonnyg but, but my gut feeling is that it's a better place to handle it all.
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.
Yes, buffer's handle should be invalidate too. I got errors during testing when first time like 'buffer object relates to another context'. And after a couple of lost/restore cycles it crashed on glDeleteBuffersARB
.
But, just as dmResource can invalidate all graphics resources, perhaps dmRender could too?
I'll think about it.
@@ -17,7 +17,7 @@ | |||
|
|||
namespace dmGameSystem | |||
{ | |||
static dmResource::Result AcquireResources(dmGraphics::HContext context, dmResource::HFactory factory, dmGraphics::ShaderDesc* ddf, dmGraphics::HVertexProgram* program) | |||
static dmResource::Result AcquireResources(dmGraphics::HContext context, dmResource::HFactory factory, dmGraphics::ShaderDesc* ddf, dmGraphics::HFragmentProgram* program) |
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.
Hehe, classic copy paste?
Technical details
render.set_listener
API.PR checklist
Example of a well written PR description:
### Technical changes
Technical changes:
Technical notes: