From 37b45a010d8d528114fded6e3f56ecd209346af1 Mon Sep 17 00:00:00 2001 From: rdb Date: Tue, 30 Nov 2021 10:56:48 +0100 Subject: [PATCH] gobj: More efficient tracking of simple image changes Remove the separate simple_image_modified field, instead, the image_modified value is updated if the simple image is changed while there is no RAM image. This robustifies the tracking on the GSG end, which previously ended up repeatedly calling upload_texture() unnecessarily --- panda/src/dxgsg9/dxGraphicsStateGuardian9.cxx | 5 +-- panda/src/dxgsg9/dxTextureContext9.cxx | 2 +- .../glstuff/glGraphicsStateGuardian_src.cxx | 22 +++++++++---- panda/src/glstuff/glTextureContext_src.I | 1 + panda/src/glstuff/glTextureContext_src.cxx | 1 + panda/src/glstuff/glTextureContext_src.h | 1 + panda/src/gobj/texture.I | 26 ++++----------- panda/src/gobj/texture.cxx | 21 ++++-------- panda/src/gobj/texture.h | 4 --- panda/src/gobj/textureContext.I | 33 ------------------- panda/src/gobj/textureContext.h | 4 --- .../tinydisplay/tinyGraphicsStateGuardian.cxx | 7 ++-- 12 files changed, 36 insertions(+), 91 deletions(-) diff --git a/panda/src/dxgsg9/dxGraphicsStateGuardian9.cxx b/panda/src/dxgsg9/dxGraphicsStateGuardian9.cxx index 35f86ae3d0..371a73f47e 100644 --- a/panda/src/dxgsg9/dxGraphicsStateGuardian9.cxx +++ b/panda/src/dxgsg9/dxGraphicsStateGuardian9.cxx @@ -357,10 +357,7 @@ upload_texture(DXTextureContext9 *dtc, bool force) { async_reload_texture(dtc); has_image = _supports_compressed_texture ? tex->has_ram_image() : tex->has_uncompressed_ram_image(); if (!has_image) { - if (dtc->was_simple_image_modified()) { - return dtc->create_simple_texture(*_screen); - } - return true; + return dtc->create_simple_texture(*_screen); } } } diff --git a/panda/src/dxgsg9/dxTextureContext9.cxx b/panda/src/dxgsg9/dxTextureContext9.cxx index 2c90a56910..19a094fc45 100644 --- a/panda/src/dxgsg9/dxTextureContext9.cxx +++ b/panda/src/dxgsg9/dxTextureContext9.cxx @@ -1157,7 +1157,7 @@ create_simple_texture(DXScreenData &scrn) { goto error_exit; } - mark_simple_loaded(); + mark_loaded(); return true; error_exit: diff --git a/panda/src/glstuff/glGraphicsStateGuardian_src.cxx b/panda/src/glstuff/glGraphicsStateGuardian_src.cxx index 30541e4a0a..61480ec021 100644 --- a/panda/src/glstuff/glGraphicsStateGuardian_src.cxx +++ b/panda/src/glstuff/glGraphicsStateGuardian_src.cxx @@ -7420,6 +7420,7 @@ framebuffer_copy_to_texture(Texture *tex, int view, int z, } gtc->_has_storage = true; + gtc->_simple_loaded = false; gtc->_uses_mipmaps = uses_mipmaps; gtc->_internal_format = internal_format; gtc->_width = width; @@ -13135,7 +13136,7 @@ apply_sampler(GLuint unit, const SamplerState &sampler, CLP(TextureContext) *gtc } } - if (sampler.uses_mipmaps() && !gtc->_uses_mipmaps && !gl_ignore_mipmaps) { + if (sampler.uses_mipmaps() && !gtc->_uses_mipmaps && !gtc->_simple_loaded && !gl_ignore_mipmaps) { // The texture wasn't created with mipmaps, but we are trying to sample it // with mipmaps. We will need to reload it. GLCAT.info() @@ -13178,10 +13179,7 @@ upload_texture(CLP(TextureContext) *gtc, bool force, bool uses_mipmaps) { async_reload_texture(gtc); has_image = _supports_compressed_texture ? tex->has_ram_image() : tex->has_uncompressed_ram_image(); if (!has_image) { - if (gtc->was_simple_image_modified()) { - return upload_simple_texture(gtc); - } - return true; + return upload_simple_texture(gtc); } } } @@ -13551,6 +13549,7 @@ upload_texture(CLP(TextureContext) *gtc, bool force, bool uses_mipmaps) { } gtc->_has_storage = true; + gtc->_simple_loaded = false; gtc->_immutable = true; gtc->_uses_mipmaps = uses_mipmaps; gtc->_internal_format = internal_format; @@ -13598,6 +13597,7 @@ upload_texture(CLP(TextureContext) *gtc, bool force, bool uses_mipmaps) { if (success) { if (needs_reload) { gtc->_has_storage = true; + gtc->_simple_loaded = false; gtc->_uses_mipmaps = uses_mipmaps; gtc->_internal_format = internal_format; gtc->_width = width; @@ -14234,7 +14234,17 @@ upload_simple_texture(CLP(TextureContext) *gtc) { width, height, 0, external_format, component_type, image_ptr); - gtc->mark_simple_loaded(); + gtc->_has_storage = true; + gtc->_simple_loaded = true; + gtc->_immutable = false; + gtc->_uses_mipmaps = false; + gtc->_internal_format = internal_format; + gtc->_width = width; + gtc->_height = height; + gtc->_depth = 1; + gtc->update_data_size_bytes(width * height * 4); + + gtc->mark_loaded(); report_my_gl_errors(); return true; diff --git a/panda/src/glstuff/glTextureContext_src.I b/panda/src/glstuff/glTextureContext_src.I index 33bda9a58f..51db7538ba 100644 --- a/panda/src/glstuff/glTextureContext_src.I +++ b/panda/src/glstuff/glTextureContext_src.I @@ -29,6 +29,7 @@ CLP(TextureContext)(CLP(GraphicsStateGuardian) *glgsg, _handle_resident = false; #endif _has_storage = false; + _simple_loaded = false; _immutable = false; _uses_mipmaps = false; _generate_mipmaps = false; diff --git a/panda/src/glstuff/glTextureContext_src.cxx b/panda/src/glstuff/glTextureContext_src.cxx index e36f8f756e..37bad3ae3a 100644 --- a/panda/src/glstuff/glTextureContext_src.cxx +++ b/panda/src/glstuff/glTextureContext_src.cxx @@ -82,6 +82,7 @@ reset_data() { _handle_resident = false; #endif _has_storage = false; + _simple_loaded = false; _immutable = false; #ifndef OPENGLES_1 diff --git a/panda/src/glstuff/glTextureContext_src.h b/panda/src/glstuff/glTextureContext_src.h index 667e356d60..41a89cb32b 100644 --- a/panda/src/glstuff/glTextureContext_src.h +++ b/panda/src/glstuff/glTextureContext_src.h @@ -64,6 +64,7 @@ public: // or glTexStorage2D() call. If none of these have changed, we can reload // the texture image with a glTexSubImage2D(). bool _has_storage; + bool _simple_loaded; bool _immutable; bool _uses_mipmaps; bool _generate_mipmaps; diff --git a/panda/src/gobj/texture.I b/panda/src/gobj/texture.I index 223a86aab5..6409ef8f26 100644 --- a/panda/src/gobj/texture.I +++ b/panda/src/gobj/texture.I @@ -28,7 +28,6 @@ make_copy() const { CDWriter cdata_tex(tex->_cycler, true); cdata_tex->inc_properties_modified(); cdata_tex->inc_image_modified(); - cdata_tex->inc_simple_image_modified(); return tex; } @@ -232,7 +231,6 @@ clear_image() { do_clear_ram_image(cdata); do_clear_simple_ram_image(cdata); cdata->inc_image_modified(); - cdata->inc_simple_image_modified(); } /** @@ -1766,6 +1764,12 @@ INLINE void Texture:: set_simple_ram_image(CPTA_uchar image, int x_size, int y_size) { CDWriter cdata(_cycler, true); do_set_simple_ram_image(cdata, image, x_size, y_size); + + // If we don't have a RAM image currently, we need to update this, to let the + // GSG know that it needs to update the simple image it is currently using. + if (!do_has_ram_image(cdata)) { + cdata->inc_image_modified(); + } } /** @@ -1797,16 +1801,6 @@ get_image_modified() const { return cdata->_image_modified; } -/** - * Returns a sequence number which is guaranteed to change at least every time - * the texture's "simple" image data is modified. - */ -INLINE UpdateSeq Texture:: -get_simple_image_modified() const { - CDReader cdata(_cycler); - return cdata->_simple_image_modified; -} - /** * Specifies the power-of-2 texture-scaling mode that will be applied to this * particular texture when it is next loaded from disk. See @@ -2466,14 +2460,6 @@ inc_image_modified() { _modified_pages[0]._modified = _image_modified; } -/** - * - */ -INLINE void Texture::CData:: -inc_simple_image_modified() { - ++_simple_image_modified; -} - /** * */ diff --git a/panda/src/gobj/texture.cxx b/panda/src/gobj/texture.cxx index 9633379a82..d065d2dcd8 100644 --- a/panda/src/gobj/texture.cxx +++ b/panda/src/gobj/texture.cxx @@ -1378,7 +1378,12 @@ new_simple_ram_image(int x_size, int y_size) { cdata->_simple_ram_image._image = PTA_uchar::empty_array(expected_page_size); cdata->_simple_ram_image._page_size = expected_page_size; cdata->_simple_image_date_generated = (int32_t)time(nullptr); - cdata->inc_simple_image_modified(); + + // If we don't have a RAM image currently, we need to update this, to let the + // GSG know that it needs to update the simple image it is currently using. + if (!do_has_ram_image(cdata)) { + cdata->inc_image_modified(); + } return cdata->_simple_ram_image._image; } @@ -5525,7 +5530,6 @@ unlocked_ensure_ram_image(bool allow_compression) { cdataw->_component_type = cdata_tex->_component_type; cdataw->inc_properties_modified(); - cdataw->inc_image_modified(); } cdataw->_keep_ram_image = cdata_tex->_keep_ram_image; @@ -5535,9 +5539,7 @@ unlocked_ensure_ram_image(bool allow_compression) { nassertr(_reloading, nullptr); _reloading = false; - // We don't generally increment the cdata->_image_modified semaphore, - // because this is just a reload, and presumably the image hasn't changed - // (unless we hit the if condition above). + cdataw->inc_image_modified(); _cvar.notify_all(); @@ -7007,7 +7009,6 @@ do_clear(CData *cdata) { cdata->inc_properties_modified(); cdata->inc_image_modified(); - cdata->inc_simple_image_modified(); } /** @@ -7696,7 +7697,6 @@ do_set_simple_ram_image(CData *cdata, CPTA_uchar image, int x_size, int y_size) cdata->_simple_ram_image._image = image.cast_non_const(); cdata->_simple_ram_image._page_size = image.size(); cdata->_simple_image_date_generated = (int32_t)time(nullptr); - cdata->inc_simple_image_modified(); } /** @@ -7791,11 +7791,6 @@ do_clear_simple_ram_image(CData *cdata) { cdata->_simple_ram_image._image.clear(); cdata->_simple_ram_image._page_size = 0; cdata->_simple_image_date_generated = 0; - - // We allow this exception: we update the _simple_image_modified here, since - // no one really cares much about that anyway, and it's convenient to do it - // here. - cdata->inc_simple_image_modified(); } /** @@ -10610,7 +10605,6 @@ do_fillin_body(CData *cdata, DatagramIterator &scan, BamReader *manager) { cdata->_simple_ram_image._image = image; cdata->_simple_ram_image._page_size = u_size; - cdata->inc_simple_image_modified(); } if (manager->get_file_minor_ver() >= 45) { @@ -10808,7 +10802,6 @@ CData(const Texture::CData ©) { _properties_modified = copy._properties_modified; _image_modified = copy._image_modified; - _simple_image_modified = copy._simple_image_modified; _modified_pages = copy._modified_pages; } diff --git a/panda/src/gobj/texture.h b/panda/src/gobj/texture.h index 3c78553c4c..de3d90e5ea 100644 --- a/panda/src/gobj/texture.h +++ b/panda/src/gobj/texture.h @@ -521,10 +521,8 @@ PUBLISHED: INLINE UpdateSeq get_properties_modified() const; INLINE UpdateSeq get_image_modified() const; - INLINE UpdateSeq get_simple_image_modified() const; MAKE_PROPERTY(properties_modified, get_properties_modified); MAKE_PROPERTY(image_modified, get_image_modified); - MAKE_PROPERTY(simple_image_modified, get_simple_image_modified); SparseArray get_image_modified_pages(UpdateSeq since, int n = 0) const; @@ -961,7 +959,6 @@ protected: INLINE void inc_properties_modified(); INLINE void inc_image_modified(); void inc_image_page_modified(int z); - INLINE void inc_simple_image_modified(); Filename _filename; Filename _alpha_filename; @@ -1030,7 +1027,6 @@ protected: UpdateSeq _properties_modified; UpdateSeq _image_modified; - UpdateSeq _simple_image_modified; ModifiedPageRanges _modified_pages; diff --git a/panda/src/gobj/textureContext.I b/panda/src/gobj/textureContext.I index a4be4a4935..70836cdb5b 100644 --- a/panda/src/gobj/textureContext.I +++ b/panda/src/gobj/textureContext.I @@ -67,15 +67,6 @@ was_image_modified() const { return _image_modified != get_texture()->get_image_modified(); } -/** - * Returns true if the texture's "simple" image has been modified since the - * last time mark_simple_loaded() was called. - */ -INLINE bool TextureContext:: -was_simple_image_modified() const { - return _simple_image_modified != get_texture()->get_simple_image_modified(); -} - /** * Returns true if the given page of the texture image has been modified since * the last time mark_loaded() was called. @@ -103,15 +94,6 @@ get_image_modified() const { return _image_modified; } -/** - * Returns a sequence number which is guaranteed to change at least every time - * the texture's "simple" image data is modified. - */ -INLINE UpdateSeq TextureContext:: -get_simple_image_modified() const { - return _simple_image_modified; -} - /** * Returns a SparseArray indicating which pages of the texture have been * modified since the last call to mark_loaded(). @@ -146,20 +128,6 @@ mark_loaded() { set_resident(true); } -/** - * Should be called after the texture's "simple" image has been loaded into - * graphics memory. - */ -INLINE void TextureContext:: -mark_simple_loaded() { - _properties_modified = get_texture()->get_properties_modified(); - _simple_image_modified = get_texture()->get_simple_image_modified(); - update_modified(std::max(_properties_modified, _simple_image_modified)); - - // The texture's not exactly resident now, but some part of it is. - set_resident(true); -} - /** * Should be called after the texture has been forced out of texture memory. */ @@ -167,7 +135,6 @@ INLINE void TextureContext:: mark_unloaded() { _properties_modified = UpdateSeq::old(); _image_modified = UpdateSeq::old(); - _simple_image_modified = UpdateSeq::old(); update_modified(UpdateSeq::old()); set_resident(false); diff --git a/panda/src/gobj/textureContext.h b/panda/src/gobj/textureContext.h index b531c49504..de828781e2 100644 --- a/panda/src/gobj/textureContext.h +++ b/panda/src/gobj/textureContext.h @@ -43,19 +43,16 @@ PUBLISHED: INLINE bool was_modified() const; INLINE bool was_properties_modified() const; INLINE bool was_image_modified() const; - INLINE bool was_simple_image_modified() const; INLINE bool was_image_page_modified(int z, int n) const; INLINE UpdateSeq get_properties_modified() const; INLINE UpdateSeq get_image_modified() const; - INLINE UpdateSeq get_simple_image_modified() const; INLINE SparseArray get_image_modified_pages(int n = 0) const; public: INLINE void update_data_size_bytes(size_t new_data_size_bytes); INLINE void mark_loaded(); - INLINE void mark_simple_loaded(); INLINE void mark_unloaded(); INLINE void mark_needs_reload(); @@ -66,7 +63,6 @@ private: int _view; UpdateSeq _properties_modified; UpdateSeq _image_modified; - UpdateSeq _simple_image_modified; public: static TypeHandle get_class_type() { diff --git a/panda/src/tinydisplay/tinyGraphicsStateGuardian.cxx b/panda/src/tinydisplay/tinyGraphicsStateGuardian.cxx index d8e80db919..8442cae415 100644 --- a/panda/src/tinydisplay/tinyGraphicsStateGuardian.cxx +++ b/panda/src/tinydisplay/tinyGraphicsStateGuardian.cxx @@ -2387,10 +2387,7 @@ upload_texture(TinyTextureContext *gtc, bool force, bool uses_mipmaps) { // meantime load a temporary simple image in its place. async_reload_texture(gtc); if (!tex->has_ram_image()) { - if (gtc->was_simple_image_modified()) { - return upload_simple_texture(gtc); - } - return true; + return upload_simple_texture(gtc); } } } @@ -2558,7 +2555,7 @@ upload_simple_texture(TinyTextureContext *gtc) { ZTextureLevel *dest = &gltex->levels[0]; memcpy(dest->pixmap, image_ptr, image_size); - gtc->mark_simple_loaded(); + gtc->mark_loaded(); return true; }