From ada4b7614137221b64a00f31fde1498064e9fb19 Mon Sep 17 00:00:00 2001 From: Mikko Rasa Date: Sat, 18 Sep 2021 12:56:18 +0300 Subject: [PATCH] Remove the separate allocation step from textures and buffers It was conceived as a way to avoid unnecessary OpenGL calls if the object is initialized by uploading full contents. However the storage extensions are almost universally supported now and data uploads are not something that usually happens every frame, so this optimization is no longer relevant. --- source/core/buffer.cpp | 39 +++++--------------- source/core/buffer.h | 5 --- source/core/framebuffer.cpp | 5 --- source/core/texture1d.cpp | 55 ++++++++-------------------- source/core/texture1d.h | 2 -- source/core/texture2d.cpp | 62 ++++++++++---------------------- source/core/texture2d.h | 5 --- source/core/texture3d.cpp | 64 ++++++++++----------------------- source/core/texture3d.h | 5 --- source/core/texturecube.cpp | 72 +++++++++---------------------------- source/core/texturecube.h | 7 ---- 11 files changed, 76 insertions(+), 245 deletions(-) diff --git a/source/core/buffer.cpp b/source/core/buffer.cpp index 71ab6e23..dcb83a7a 100644 --- a/source/core/buffer.cpp +++ b/source/core/buffer.cpp @@ -17,8 +17,7 @@ namespace GL { Buffer *Buffer::scratch_binding = 0; Buffer::Buffer(): - size(0), - allocated(false) + size(0) { static Require _req(ARB_vertex_buffer_object); @@ -47,14 +46,6 @@ void Buffer::storage(unsigned sz) throw invalid_argument("Buffer::storage"); size = sz; -} - -void Buffer::allocate() -{ - if(size==0) - throw invalid_operation("Buffer::allocate"); - if(allocated) - return; if(ARB_buffer_storage) { @@ -66,30 +57,19 @@ void Buffer::allocate() bind_scratch(); glBufferStorage(GL_ARRAY_BUFFER, size, 0, flags); } - - allocated = true; } - else - data(0); -} - -void Buffer::data(const void *d) -{ - if(size==0) - throw invalid_operation("Buffer::data"); - - if(ARB_buffer_storage) - return sub_data(0, size, d); - - if(ARB_direct_state_access) - glNamedBufferData(id, size, d, GL_STATIC_DRAW); + else if(ARB_direct_state_access) + glNamedBufferData(id, size, 0, GL_STATIC_DRAW); else { bind_scratch(); - glBufferData(GL_ARRAY_BUFFER, size, d, GL_STATIC_DRAW); + glBufferData(GL_ARRAY_BUFFER, size, 0, GL_STATIC_DRAW); } +} - allocated = true; +void Buffer::data(const void *d) +{ + return sub_data(0, size, d); } void Buffer::sub_data(unsigned off, unsigned sz, const void *d) @@ -97,8 +77,6 @@ void Buffer::sub_data(unsigned off, unsigned sz, const void *d) if(size==0) throw invalid_operation("Buffer::sub_data"); - allocate(); - if(ARB_direct_state_access) glNamedBufferSubData(id, off, sz, d); else @@ -118,7 +96,6 @@ void *Buffer::map() { static Require _req(ARB_map_buffer_range); - allocate(); if(ARB_direct_state_access) return glMapNamedBufferRange(id, 0, size, GL_MAP_READ_BIT|GL_MAP_WRITE_BIT); else diff --git a/source/core/buffer.h b/source/core/buffer.h index 0d656131..9144afae 100644 --- a/source/core/buffer.h +++ b/source/core/buffer.h @@ -28,7 +28,6 @@ class Buffer private: unsigned id; unsigned size; - bool allocated; static Buffer *scratch_binding; @@ -43,10 +42,6 @@ public: be uploaded. Storage cannot be changed once set. */ void storage(unsigned); - /** Allocates storage for the buffer. The contents are initially undefined. - If storage has already been allocated, does nothing. */ - void allocate(); - /** Uploads data into the buffer, completely replacing any previous contents. Storage must be defined beforehand. The data must have size matching the defined storage. */ diff --git a/source/core/framebuffer.cpp b/source/core/framebuffer.cpp index 851b6431..d6fa9739 100644 --- a/source/core/framebuffer.cpp +++ b/source/core/framebuffer.cpp @@ -273,7 +273,6 @@ void Framebuffer::set_attachment(FrameAttachment attch, Texture &tex, unsigned l void Framebuffer::attach(FrameAttachment attch, Texture2D &tex, unsigned level) { - tex.allocate(level); set_attachment(make_typed_attachment(attch, tex.get_format()), tex, level, 0, 0); } @@ -284,27 +283,23 @@ void Framebuffer::attach(FrameAttachment attch, Texture2DMultisample &tex) void Framebuffer::attach(FrameAttachment attch, Texture3D &tex, unsigned layer, unsigned level) { - tex.allocate(level); set_attachment(make_typed_attachment(attch, tex.get_format()), tex, level, layer, 0); } void Framebuffer::attach(FrameAttachment attch, TextureCube &tex, TextureCubeFace face, unsigned level) { - tex.allocate(level); set_attachment(make_typed_attachment(attch, tex.get_format()), tex, level, TextureCube::get_face_index(face), 0); } void Framebuffer::attach_layered(FrameAttachment attch, Texture3D &tex, unsigned level) { static Require _req(ARB_geometry_shader4); - tex.allocate(level); set_attachment(make_typed_attachment(attch, tex.get_format()), tex, level, -1, 0); } void Framebuffer::attach_layered(FrameAttachment attch, TextureCube &tex, unsigned level) { static Require _req(ARB_geometry_shader4); - tex.allocate(level); set_attachment(make_typed_attachment(attch, tex.get_format()), tex, level, -1, 0); } diff --git a/source/core/texture1d.cpp b/source/core/texture1d.cpp index 7c3f02ab..3fd1ae09 100644 --- a/source/core/texture1d.cpp +++ b/source/core/texture1d.cpp @@ -11,8 +11,7 @@ namespace GL { Texture1D::Texture1D(): Texture(GL_TEXTURE_1D), - width(0), - allocated(0) + width(0) { static Require _req(MSP_texture1D); } @@ -33,58 +32,34 @@ void Texture1D::storage(PixelFormat fmt, unsigned wd, unsigned lv) levels = get_n_levels(); if(lv) levels = min(levels, lv); -} - -void Texture1D::allocate(unsigned level) -{ - if(width==0) - throw invalid_operation("Texture1D::allocate"); - if(level>=levels) - throw invalid_argument("Texture1D::allocate"); - if(allocated&(1<=levels) - throw out_of_range("Texture1D::image"); - - if(ARB_texture_storage) - return sub_image(level, 0, get_level_size(level), data); - - bind_scratch(); - - if(!allocated) { + bind_scratch(); glTexParameteri(target, GL_TEXTURE_MAX_LEVEL, levels-1); - apply_swizzle(); + GLenum comp = get_gl_components(get_components(storage_fmt)); + GLenum type = get_gl_type(get_component_type(storage_fmt)); + for(unsigned i=0; i=levels) throw out_of_range("Texture1D::sub_image"); - allocate(level); - GLenum comp = get_gl_components(get_components(storage_fmt)); GLenum type = get_gl_type(get_component_type(storage_fmt)); if(ARB_direct_state_access) diff --git a/source/core/texture1d.h b/source/core/texture1d.h index d8dbb017..241808c1 100644 --- a/source/core/texture1d.h +++ b/source/core/texture1d.h @@ -25,14 +25,12 @@ public: private: unsigned width; unsigned levels; - unsigned allocated; public: Texture1D(); void storage(PixelFormat, unsigned, unsigned = 0); - void allocate(unsigned); void image(unsigned, const void *); void sub_image(unsigned, int, unsigned, const void *); virtual void image(const Graphics::Image &, unsigned = 0); diff --git a/source/core/texture2d.cpp b/source/core/texture2d.cpp index b5b989a8..31d70165 100644 --- a/source/core/texture2d.cpp +++ b/source/core/texture2d.cpp @@ -36,8 +36,7 @@ public: Texture2D::Texture2D(ResourceManager *m): Texture(GL_TEXTURE_2D, m), width(0), - height(0), - allocated(0) + height(0) { } Texture2D::~Texture2D() @@ -62,60 +61,38 @@ void Texture2D::storage(PixelFormat fmt, unsigned wd, unsigned ht, unsigned lv) levels = get_n_levels(); if(lv>0) levels = min(levels, lv); -} - -void Texture2D::allocate(unsigned level) -{ - if(width==0 || height==0) - throw invalid_operation("Texture2D::allocate"); - if(level>=levels) - throw invalid_argument("Texture2D::allocate"); - if(allocated&(1<=levels) - throw out_of_range("Texture2D::image"); - - LinAl::Vector size = get_level_size(level); - - if(ARB_texture_storage) - return sub_image(level, 0, 0, size.x, size.y, data); - - bind_scratch(); - - if(!allocated) { + bind_scratch(); glTexParameteri(target, GL_TEXTURE_MAX_LEVEL, levels-1); - apply_swizzle(); + GLenum comp = get_gl_components(get_components(storage_fmt)); + GLenum type = get_gl_type(get_component_type(storage_fmt)); + for(unsigned i=0; i lv_size = get_level_size(i); + glTexImage2D(target, i, gl_fmt, lv_size.x, lv_size.y, 0, comp, type, 0); + } } - GLenum fmt = get_gl_pixelformat(storage_fmt); - GLenum comp = get_gl_components(get_components(storage_fmt)); - GLenum type = get_gl_type(get_component_type(storage_fmt)); - glTexImage2D(target, level, fmt, size.x, size.y, 0, comp, type, data); + apply_swizzle(); +} - allocated |= 1< size = get_level_size(level); + return sub_image(level, 0, 0, size.x, size.y, data); } void Texture2D::sub_image(unsigned level, int x, int y, unsigned wd, unsigned ht, const void *data) @@ -125,8 +102,6 @@ void Texture2D::sub_image(unsigned level, int x, int y, unsigned wd, unsigned ht if(level>=levels) throw out_of_range("Texture2D::sub_image"); - allocate(level); - GLenum comp = get_gl_components(get_components(storage_fmt)); GLenum type = get_gl_type(get_component_type(storage_fmt)); if(ARB_direct_state_access) @@ -188,7 +163,6 @@ void Texture2D::unload() { glDeleteTextures(1, &id); id = 0; - allocated = 0; } diff --git a/source/core/texture2d.h b/source/core/texture2d.h index b3a5a749..eba553b0 100644 --- a/source/core/texture2d.h +++ b/source/core/texture2d.h @@ -36,7 +36,6 @@ private: unsigned width; unsigned height; unsigned levels; - unsigned allocated; public: Texture2D(ResourceManager * = 0); @@ -49,10 +48,6 @@ public: it can't be changed. */ void storage(PixelFormat fmt, unsigned wd, unsigned ht, unsigned lv = 0); - /** Allocates storage for the texture. The contents are initially - undefined. If storage has already been allocated, does nothing. */ - void allocate(unsigned level); - /** Updates the contents of the entire texture. Storage must be defined beforehand. The image data must have dimensions and format matching the defined storage. */ diff --git a/source/core/texture3d.cpp b/source/core/texture3d.cpp index 984ffac7..03a64f1c 100644 --- a/source/core/texture3d.cpp +++ b/source/core/texture3d.cpp @@ -16,16 +16,14 @@ Texture3D::Texture3D(GLenum t): Texture(t), width(0), height(0), - depth(0), - allocated(0) + depth(0) { } Texture3D::Texture3D(): Texture(GL_TEXTURE_3D), width(0), height(0), - depth(0), - allocated(0) + depth(0) { static Require _req(EXT_texture3D); } @@ -48,60 +46,38 @@ void Texture3D::storage(PixelFormat fmt, unsigned wd, unsigned ht, unsigned dp, levels = get_n_levels(); if(lv>0) levels = min(levels, lv); -} - -void Texture3D::allocate(unsigned level) -{ - if(width==0 || height==0 || depth==0) - throw invalid_operation("Texture3D::allocate"); - if(level>=levels) - throw invalid_argument("Texture3D::allocate"); - if(allocated&(1<=levels) - throw out_of_range("Texture3D::image"); - - LinAl::Vector size = get_level_size(level); - - if(ARB_texture_storage) - return sub_image(level, 0, 0, 0, size.x, size.y, size.z, data); - - bind_scratch(); - - if(!allocated) { + bind_scratch(); + GLenum comp = get_gl_components(get_components(storage_fmt)); + GLenum type = get_gl_type(get_component_type(storage_fmt)); + for(unsigned i=0; i lv_size = get_level_size(i); + glTexImage3D(target, i, gl_fmt, lv_size.x, lv_size.y, lv_size.z, 0, comp, type, 0); + } glTexParameteri(target, GL_TEXTURE_MAX_LEVEL, levels-1); - apply_swizzle(); } - GLenum fmt = get_gl_pixelformat(storage_fmt); - GLenum comp = get_gl_components(get_components(storage_fmt)); - GLenum type = get_gl_type(get_component_type(storage_fmt)); - glTexImage3D(target, level, fmt, size.x, size.y, size.z, 0, comp, type, data); + apply_swizzle(); +} - allocated |= 1< size = get_level_size(level); + return sub_image(level, 0, 0, 0, size.x, size.y, size.z, data); } void Texture3D::sub_image(unsigned level, int x, int y, int z, unsigned wd, unsigned ht, unsigned dp, const void *data) @@ -111,8 +87,6 @@ void Texture3D::sub_image(unsigned level, int x, int y, int z, unsigned wd, unsi if(level>=levels) throw out_of_range("Texture3D::sub_image"); - allocate(level); - GLenum comp = get_gl_components(get_components(storage_fmt)); GLenum type = get_gl_type(get_component_type(storage_fmt)); if(ARB_direct_state_access) diff --git a/source/core/texture3d.h b/source/core/texture3d.h index 3538f6bb..7b88db50 100644 --- a/source/core/texture3d.h +++ b/source/core/texture3d.h @@ -33,7 +33,6 @@ protected: unsigned height; unsigned depth; unsigned levels; - unsigned allocated; Texture3D(GLenum); public: @@ -46,10 +45,6 @@ public: it can't be changed. */ void storage(PixelFormat fmt, unsigned wd, unsigned ht, unsigned dp, unsigned lv = 0); - /** Allocates storage for the texture. The contents are initially - undefined. If storage has already been allocated, does nothing. */ - void allocate(unsigned level); - /** Updates the contents of the entire texture. Storage must be defined beforehand. The image data must have dimensions and format matching the defined storage. */ diff --git a/source/core/texturecube.cpp b/source/core/texturecube.cpp index 4bbf7e6f..5ee772ca 100644 --- a/source/core/texturecube.cpp +++ b/source/core/texturecube.cpp @@ -45,8 +45,7 @@ unsigned TextureCube::orientations[12] = TextureCube::TextureCube(): Texture(GL_TEXTURE_CUBE_MAP), - size(0), - allocated(0) + size(0) { static Require _req(ARB_texture_cube_map); if(ARB_seamless_cube_map) @@ -69,74 +68,39 @@ void TextureCube::storage(PixelFormat fmt, unsigned sz, unsigned lv) levels = get_n_levels(); if(lv>0) levels = min(levels, lv); -} - -void TextureCube::allocate(unsigned level) -{ - if(size==0) - throw invalid_operation("TextureCube::allocate"); - if(level>=levels) - throw invalid_argument("TextureCube::allocate"); - if(allocated&(64<=levels) - throw out_of_range("TextureCube::image"); - unsigned lsz = get_level_size(level); - - if(ARB_texture_storage) - return sub_image(face, level, 0, 0, lsz, lsz, data); - - if(!allocated) - { - glTexParameteri(target, GL_TEXTURE_MAX_LEVEL, levels-1); - apply_swizzle(); - } - - GLenum fmt = get_gl_pixelformat(storage_fmt); - GLenum comp = get_gl_components(get_components(storage_fmt)); - GLenum type = get_gl_type(get_component_type(storage_fmt)); - glTexImage2D(face, level, fmt, lsz, lsz, 0, comp, type, data); - - if(level==0) - { - allocated |= 1<=levels) throw out_of_range("TextureCube::sub_image"); - allocate(level); - GLenum comp = get_gl_components(get_components(storage_fmt)); GLenum type = get_gl_type(get_component_type(storage_fmt)); if(ARB_direct_state_access) diff --git a/source/core/texturecube.h b/source/core/texturecube.h index 93c14336..b68210b0 100644 --- a/source/core/texturecube.h +++ b/source/core/texturecube.h @@ -52,9 +52,6 @@ public: private: unsigned size; unsigned levels; - /* Lowest six bits track allocation status of faces on the base level. Bit - seven is set if the entire base level is allocated. */ - unsigned allocated; static TextureCubeFace face_order[6]; static Vector3 directions[6]; @@ -70,10 +67,6 @@ public: it can't be changed. */ void storage(PixelFormat fmt, unsigned size, unsigned lv = 0); - /** Allocates storage for the cube faces. The contents are initially - undefined. If storage has already been allocated, does nothing. */ - void allocate(unsigned level); - /** Updates the contents of a face. Storage must be defined beforehand. The image data must have dimensions and format matching the defined storage. */ -- 2.43.0