From 9ec0e018234391efa66cc0f4080bfe470b910288 Mon Sep 17 00:00:00 2001 From: Mikko Rasa Date: Sat, 13 Nov 2021 16:56:32 +0200 Subject: [PATCH] Replace ProgramData copy constructor with a more explicit copy function The copy constructor left so many members uncopied that it was rather questionable as copy constructor. This makes it more clear that only uniform values are being copied. --- demos/desertpillars/source/desertpillars.cpp | 2 +- source/materials/rendermethod.cpp | 26 +++++++++-- source/materials/technique.cpp | 5 ++- source/render/camera.cpp | 17 ++++++++ source/render/camera.h | 2 + source/render/programdata.cpp | 46 +++----------------- source/render/programdata.h | 7 ++- 7 files changed, 55 insertions(+), 50 deletions(-) diff --git a/demos/desertpillars/source/desertpillars.cpp b/demos/desertpillars/source/desertpillars.cpp index dd801e29..62a44558 100644 --- a/demos/desertpillars/source/desertpillars.cpp +++ b/demos/desertpillars/source/desertpillars.cpp @@ -26,7 +26,6 @@ DesertPillars::DesertPillars(int, char **): keyboard(window), resources(&res_mgr), view(window), - camera(resources.get("Camera.camera")), lighting(resources.get("Desert.lightn")), sphere(resources.get("Sphere.object")), sphere_morph(0.0f), @@ -60,6 +59,7 @@ DesertPillars::DesertPillars(int, char **): content.add(*env_map); content.add(flare); + camera.copy_parameters(resources.get("Camera.camera")); camera.set_debug_name("Main camera"); view.set_content(sequence.get()); diff --git a/source/materials/rendermethod.cpp b/source/materials/rendermethod.cpp index 0302a993..5c838ef9 100644 --- a/source/materials/rendermethod.cpp +++ b/source/materials/rendermethod.cpp @@ -38,7 +38,11 @@ void RenderMethod::maybe_create_material_shader() shprog = material->create_compatible_shader(extra_spec); if(shdata) - shdata = new ProgramData(*shdata, shprog); + { + RefPtr old_shdata = shdata; + shdata = new ProgramData(shprog); + shdata->copy_uniforms(*old_shdata); + } shprog_from_material = true; } @@ -47,7 +51,13 @@ void RenderMethod::set_shader_program(const Program *prog, const ProgramData *da { shprog = prog; shprog_from_material = false; - shdata = (data ? new ProgramData(*data) : 0); + if(data) + { + shdata = new ProgramData; + shdata->copy_uniforms(*data); + } + else + shdata = 0; maybe_create_material_shader(); } @@ -194,7 +204,11 @@ void RenderMethod::Loader::shader(const string &n) obj.shprog = &get_collection().get(n); obj.shprog_from_material = false; if(obj.shdata) - obj.shdata = new ProgramData(*obj.shdata, obj.shprog); + { + RefPtr old_shdata = obj.shdata; + obj.shdata = new ProgramData(obj.shprog); + obj.shdata->copy_uniforms(*old_shdata); + } } void RenderMethod::Loader::texture(const string &n) @@ -216,7 +230,11 @@ void RenderMethod::Loader::uniforms() if(!obj.shdata) obj.shdata = new ProgramData(obj.shprog); else if(obj.shdata.refcount()>1) - obj.shdata = new ProgramData(*obj.shdata); + { + RefPtr old_shdata = obj.shdata; + obj.shdata = new ProgramData; + obj.shdata->copy_uniforms(*old_shdata); + } load_sub(*obj.shdata); } diff --git a/source/materials/technique.cpp b/source/materials/technique.cpp index 60eba997..163cbec6 100644 --- a/source/materials/technique.cpp +++ b/source/materials/technique.cpp @@ -79,7 +79,10 @@ bool Technique::replace_uniforms(const ProgramData &shdata) continue; if(!new_shdata) - new_shdata = new ProgramData(*kvp.second.get_shader_data()); + { + new_shdata = new ProgramData; + new_shdata->copy_uniforms(*kvp.second.get_shader_data()); + } new_shdata->copy_uniform(shdata, tag); replaced = true; diff --git a/source/render/camera.cpp b/source/render/camera.cpp index 8f681a74..3ace92b9 100644 --- a/source/render/camera.cpp +++ b/source/render/camera.cpp @@ -13,6 +13,23 @@ Camera::Camera() update_object_matrix(); } +void Camera::copy_parameters(const Camera &source) +{ + fov = source.fov; + height = source.height; + aspect = source.aspect; + clip_near = source.clip_near; + clip_far = source.clip_far; + frustum_x = source.frustum_x; + frustum_y = source.frustum_y; + rotate = source.rotate; + position = source.position; + look_dir = source.look_dir; + up_dir = source.up_dir; + update_projection_matrix(); + update_object_matrix(); +} + void Camera::set_field_of_view(const Geometry::Angle &f) { fov = f; diff --git a/source/render/camera.h b/source/render/camera.h index ed81a951..cb9b4ce1 100644 --- a/source/render/camera.h +++ b/source/render/camera.h @@ -61,6 +61,8 @@ private: public: Camera(); + void copy_parameters(const Camera &); + /** Sets the camera projection to perspective, characterised by the vertical field of view. Horizontal FoV is computed with the aspect ratio. */ void set_field_of_view(const Geometry::Angle &); diff --git a/source/render/programdata.cpp b/source/render/programdata.cpp index cfd2c0f5..a908284b 100644 --- a/source/render/programdata.cpp +++ b/source/render/programdata.cpp @@ -20,46 +20,6 @@ ProgramData::ProgramData(const Program *p): tied_program(p) { } -// Blocks are intentionally left uncopied -ProgramData::ProgramData(const ProgramData &other): - tied_program(other.tied_program), - uniforms(other.uniforms), - uniform_data(other.uniform_data), - generation(other.generation) -{ } - -ProgramData::ProgramData(const ProgramData &other, const Program *p): - tied_program(p) -{ - if(tied_program) - { - for(const TaggedUniform &u: other.uniforms) - validate_tag(u.tag); - } - - uniforms = other.uniforms; - uniform_data = other.uniform_data; -} - -ProgramData &ProgramData::operator=(const ProgramData &other) -{ - tied_program = other.tied_program; - - uniforms = other.uniforms; - uniform_data = other.uniform_data; - - for(SharedBlock &b: blocks) - delete b.block; - blocks.clear(); - programs.clear(); - - last_buffer_block = 0; - buffer = 0; - dirty = 0; - - return *this; -} - ProgramData::ProgramData(ProgramData &&other): tied_program(other.tied_program), uniforms(move(other.uniforms)), @@ -482,6 +442,12 @@ void ProgramData::copy_uniform(const ProgramData &source, Tag tag) uniform(tag, tu.type, tu.array_size, source.uniform_data.data()+tu.data_offset); } +void ProgramData::copy_uniforms(const ProgramData &source) +{ + for(const TaggedUniform &u: source.uniforms) + uniform(u.tag, u.type, u.array_size, source.uniform_data.data()+u.data_offset); +} + int ProgramData::find_uniform_index(Tag tag) const { auto i = lower_bound_member(uniforms, tag, &TaggedUniform::tag); diff --git a/source/render/programdata.h b/source/render/programdata.h index 53f48a6b..f1c5996e 100644 --- a/source/render/programdata.h +++ b/source/render/programdata.h @@ -2,6 +2,7 @@ #define MSP_GL_PROGRAMDATA_H_ #include +#include #include #include "datatype.h" #include "matrix.h" @@ -35,7 +36,7 @@ programs have the same layout, the same block is used for them. The class is optimized for an access pattern where the set of uniforms and programs stays constants, with only the values changing. */ -class ProgramData +class ProgramData: public NonCopyable { public: class Loader: public DataFile::ObjectLoader @@ -162,9 +163,6 @@ private: public: ProgramData(const Program * = 0); - ProgramData(const ProgramData &); - ProgramData(const ProgramData &, const Program *); - ProgramData &operator=(const ProgramData &); ProgramData(ProgramData &&); ~ProgramData(); @@ -248,6 +246,7 @@ public: std::vector get_uniform_tags() const; void copy_uniform(const ProgramData &, Tag); + void copy_uniforms(const ProgramData &); private: int find_uniform_index(Tag) const; -- 2.43.0