From f7fde882b8a27a7319690910fb514ed65710b6d7 Mon Sep 17 00:00:00 2001 From: rdb Date: Sun, 10 Jul 2016 12:25:35 +0200 Subject: [PATCH 1/4] Fix uninitialised variable --- panda/src/display/graphicsStateGuardian.cxx | 1 + 1 file changed, 1 insertion(+) diff --git a/panda/src/display/graphicsStateGuardian.cxx b/panda/src/display/graphicsStateGuardian.cxx index 22a2caf498..0a2ae03c1e 100644 --- a/panda/src/display/graphicsStateGuardian.cxx +++ b/panda/src/display/graphicsStateGuardian.cxx @@ -235,6 +235,7 @@ GraphicsStateGuardian(CoordinateSystem internal_coordinate_system, _supports_basic_shaders = false; _supports_geometry_shaders = false; _supports_tessellation_shaders = false; + _supports_compute_shaders = false; _supports_glsl = false; _supports_stencil = false; From fa7730819f3c57a1696a094fc6a6a87e842ed503 Mon Sep 17 00:00:00 2001 From: rdb Date: Sun, 10 Jul 2016 12:28:11 +0200 Subject: [PATCH 2/4] Fix assertion when using aux render targets in DX9 --- doc/ReleaseNotes | 2 + panda/src/dxgsg9/wdxGraphicsBuffer9.cxx | 6 ++- panda/src/gobj/shader.cxx | 63 +++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 2 deletions(-) diff --git a/doc/ReleaseNotes b/doc/ReleaseNotes index 26ab1d8813..83d77d4aef 100644 --- a/doc/ReleaseNotes +++ b/doc/ReleaseNotes @@ -21,6 +21,8 @@ This issue fixes several bugs that were still found in 1.9.2. * Fix cull issue when rendering cube map (or any multi-lens setup) * Fix crash rendering with the same camera to different contexts * Fix compile error when making static build with DX9 renderer +* Fix assertion when using aux render targets in DX9 +* Work around Cg bug generating invalid ASM for saturated tex loads ------------------------ RELEASE 1.9.2 ------------------------ diff --git a/panda/src/dxgsg9/wdxGraphicsBuffer9.cxx b/panda/src/dxgsg9/wdxGraphicsBuffer9.cxx index f393987726..9f04ea7a25 100644 --- a/panda/src/dxgsg9/wdxGraphicsBuffer9.cxx +++ b/panda/src/dxgsg9/wdxGraphicsBuffer9.cxx @@ -329,16 +329,18 @@ rebuild_bitplanes() { case RTP_aux_float_3: { CDWriter cdataw(_cycler, cdata, false); - nassertr(cdata->_textures.size() == cdataw->_textures.size(), false); cdataw->_textures[i]._rtm_mode = RTM_none; } + // Creating the CDWriter invalidated the CDLockedReader. + cdata = CDLockedReader(_cycler); break; default: { CDWriter cdataw(_cycler, cdata, false); - nassertr(cdata->_textures.size() == cdataw->_textures.size(), false); cdataw->_textures[i]._rtm_mode = RTM_copy_texture; } + // Creating the CDWriter invalidated the CDLockedReader. + cdata = CDLockedReader(_cycler); break; } } diff --git a/panda/src/gobj/shader.cxx b/panda/src/gobj/shader.cxx index b4b628744a..448fc3c25a 100644 --- a/panda/src/gobj/shader.cxx +++ b/panda/src/gobj/shader.cxx @@ -1620,6 +1620,69 @@ cg_compile_shader(const ShaderCaps &caps, CGcontext context) { return false; } + // This is present to work around a bug in the Cg compiler for Direct3D 9. + // It generates "texld_sat" instructions that the result in an + // D3DXERR_INVALIDDATA error when trying to load the shader, since the _sat + // modifier may not be used on tex* instructions. + if (_cg_fprofile == CG_PROFILE_PS_2_0 || + _cg_fprofile == CG_PROFILE_PS_2_X || + _cg_fprofile == CG_PROFILE_PS_3_0) { + vector_string lines; + tokenize(cgGetProgramString(_cg_fprogram, CG_COMPILED_PROGRAM), lines, "\n"); + + ostringstream out; + int num_modified = 0; + + for (size_t i = 0; i < lines.size(); ++i) { + const string &line = lines[i]; + + size_t space = line.find(' '); + if (space == string::npos) { + out << line << '\n'; + continue; + } + + string instr = line.substr(0, space); + + // Look for a texld instruction with _sat modifier. + if (instr.compare(0, 5, "texld") == 0 && + instr.compare(instr.size() - 4, 4, "_sat") == 0) { + // Which destination register are we operating on? + string reg = line.substr(space + 1, line.find(',', space) - space - 1); + + // Move the saturation operation to a separate instruction. + instr.resize(instr.size() - 4); + out << instr << ' ' << line.substr(space + 1) << '\n'; + out << "mov_sat " << reg << ", " << reg << '\n'; + ++num_modified; + } else { + out << line << '\n'; + } + } + + if (num_modified > 0) { + string result = out.str(); + CGprogram new_program; + new_program = cgCreateProgram(context, CG_OBJECT, result.c_str(), + (CGprofile)_cg_fprofile, "fshader", + (const char**)NULL); + if (new_program) { + cgDestroyProgram(_cg_fprogram); + _cg_fprogram = new_program; + + if (shader_cat.is_debug()) { + shader_cat.debug() + << "Replaced " << num_modified << " invalid texld_sat instruction" + << ((num_modified == 1) ? "" : "s") << " in compiled shader\n"; + } + } else { + shader_cat.warning() + << "Failed to load shader with fixed texld_sat instructions: " + << cgGetErrorString(cgGetError()) << "\n"; + } + } + } + // DEBUG: output the generated program if (shader_cat.is_debug()) { const char *vertex_program; From c34758ea5c9835a78c65e25fcd9551e29f8e7cb2 Mon Sep 17 00:00:00 2001 From: rdb Date: Sun, 10 Jul 2016 14:06:01 +0200 Subject: [PATCH 3/4] Cg fixes for DX9 --- doc/ReleaseNotes | 1 + panda/src/dxgsg9/dxGraphicsStateGuardian9.cxx | 29 ++++- panda/src/dxgsg9/dxShaderContext9.cxx | 101 ++++++++++-------- panda/src/pgraph/shaderInput.I | 12 ++- 4 files changed, 92 insertions(+), 51 deletions(-) diff --git a/doc/ReleaseNotes b/doc/ReleaseNotes index 83d77d4aef..7ffdb5ecc7 100644 --- a/doc/ReleaseNotes +++ b/doc/ReleaseNotes @@ -23,6 +23,7 @@ This issue fixes several bugs that were still found in 1.9.2. * Fix compile error when making static build with DX9 renderer * Fix assertion when using aux render targets in DX9 * Work around Cg bug generating invalid ASM for saturated tex loads +* Fix issues with certain Cg shader inputs in DX9 ------------------------ RELEASE 1.9.2 ------------------------ diff --git a/panda/src/dxgsg9/dxGraphicsStateGuardian9.cxx b/panda/src/dxgsg9/dxGraphicsStateGuardian9.cxx index cafc199a95..c5d71e152d 100644 --- a/panda/src/dxgsg9/dxGraphicsStateGuardian9.cxx +++ b/panda/src/dxgsg9/dxGraphicsStateGuardian9.cxx @@ -431,10 +431,35 @@ extract_texture_data(Texture *tex) { //////////////////////////////////////////////////////////////////// ShaderContext *DXGraphicsStateGuardian9:: prepare_shader(Shader *se) { + PStatTimer timer(_prepare_shader_pcollector); + + switch (se->get_language()) { + case Shader::SL_GLSL: + dxgsg9_cat.error() + << "Tried to load GLSL shader, but GLSL shaders not supported by Direct3D 9.\n"; + return NULL; + + case Shader::SL_Cg: #ifdef HAVE_CG - CLP(ShaderContext) *result = new CLP(ShaderContext)(se, this); - return result; + if (_supports_basic_shaders) { + return new CLP(ShaderContext)(se, this); + } else { + dxgsg9_cat.error() + << "Tried to load Cg shader, but basic shaders not supported.\n"; + return NULL; + } +#else + dxgsg9_cat.error() + << "Tried to load Cg shader, but Cg support not compiled in.\n"; + return NULL; #endif + + default: + dxgsg9_cat.error() + << "Tried to load shader with unsupported shader language!\n"; + return NULL; + } + return NULL; } diff --git a/panda/src/dxgsg9/dxShaderContext9.cxx b/panda/src/dxgsg9/dxShaderContext9.cxx index 0e86d46c16..c41f686526 100644 --- a/panda/src/dxgsg9/dxShaderContext9.cxx +++ b/panda/src/dxgsg9/dxShaderContext9.cxx @@ -49,7 +49,6 @@ CLP(ShaderContext)(Shader *s, GSG *gsg) : ShaderContext(s) { CGcontext context = DCAST(DXGraphicsStateGuardian9, gsg)->_cg_context; if (s->get_language() == Shader::SL_Cg) { - // Ask the shader to compile itself for us and // to give us the resulting Cg program objects. if (!s->cg_compile_for(gsg->_shader_caps, context, @@ -259,43 +258,54 @@ issue_parameters(GSG *gsg, int altered) { #ifdef HAVE_CG if (_cg_program) { - // Iterate through _ptr parameters - for (int i=0; i<(int)_shader->_ptr_spec.size(); i++) { - if(altered & (_shader->_ptr_spec[i]._dep[0] | _shader->_ptr_spec[i]._dep[1])){ -#ifdef HAVE_CG - const Shader::ShaderPtrSpec& _ptr = _shader->_ptr_spec[i]; - Shader::ShaderPtrData* _ptr_data = - const_cast< Shader::ShaderPtrData*>(gsg->fetch_ptr_parameter(_ptr)); + // Iterate through _ptr parameters + for (size_t i = 0; i < _shader->_ptr_spec.size(); ++i) { + const Shader::ShaderPtrSpec &spec = _shader->_ptr_spec[i]; - if (_ptr_data == NULL){ //the input is not contained in ShaderPtrData + if (altered & (spec._dep[0] | spec._dep[1])) { + const Shader::ShaderPtrData *ptr_data = gsg->fetch_ptr_parameter(spec); + + if (ptr_data == NULL) { //the input is not contained in ShaderPtrData release_resources(); return; } - CGparameter p = _cg_parameter_map[_ptr._id._seqno]; + // Calculate how many elements to transfer; no more than it expects, + // but certainly no more than we have. + int input_size = min(abs(spec._dim[0] * spec._dim[1] * spec._dim[2]), ptr_data->_size); + + CGparameter p = _cg_parameter_map[spec._id._seqno]; + switch (ptr_data->_type) { + case Shader::SPT_int: + cgSetParameterValueic(p, input_size, (int *)ptr_data->_ptr); + break; + + case Shader::SPT_double: + cgSetParameterValuedc(p, input_size, (double *)ptr_data->_ptr); + break; - switch(_ptr_data->_type) { case Shader::SPT_float: - cgD3D9SetUniform(p, (PN_stdfloat*)_ptr_data->_ptr); + cgSetParameterValuefc(p, input_size, (float *)ptr_data->_ptr); break; default: dxgsg9_cat.error() - << _ptr._id._name << ":" << "unrecognized parameter type\n"; + << spec._id._name << ": unrecognized parameter type\n"; release_resources(); return; } } -#endif } - for (int i=0; i<(int)_shader->_mat_spec.size(); i++) { - if (altered & (_shader->_mat_spec[i]._dep[0] | _shader->_mat_spec[i]._dep[1])) { - CGparameter p = _cg_parameter_map[_shader->_mat_spec[i]._id._seqno]; + for (size_t i = 0; i < _shader->_mat_spec.size(); ++i) { + Shader::ShaderMatSpec &spec = _shader->_mat_spec[i]; + + if (altered & (spec._dep[0] | spec._dep[1])) { + CGparameter p = _cg_parameter_map[spec._id._seqno]; if (p == NULL) { continue; } - const LMatrix4 *val = gsg->fetch_specified_value(_shader->_mat_spec[i], altered); + const LMatrix4 *val = gsg->fetch_specified_value(spec, altered); if (val) { HRESULT hr; PN_stdfloat v [4]; @@ -309,12 +319,12 @@ issue_parameters(GSG *gsg, int altered) { #if DEBUG_SHADER // DEBUG global_data = (PN_stdfloat *) data; - global_shader_mat_spec = &_shader->_mat_spec[i]; + global_shader_mat_spec = &spec; global_internal_name_0 = global_shader_mat_spec -> _arg [0]; global_internal_name_1 = global_shader_mat_spec -> _arg [1]; #endif - switch (_shader->_mat_spec[i]._piece) { + switch (spec._piece) { case Shader::SMP_whole: // TRANSPOSE REQUIRED temp_matrix.transpose_in_place(); @@ -363,26 +373,22 @@ issue_parameters(GSG *gsg, int altered) { default: dxgsg9_cat.error() - << "issue_parameters ( ) SMP parameter type not implemented " << _shader->_mat_spec[i]._piece << "\n"; + << "issue_parameters ( ) SMP parameter type not implemented " << spec._piece << "\n"; break; } if (FAILED (hr)) { - string name = "unnamed"; - if (_shader->_mat_spec[i]._arg [0]) { - name = _shader->_mat_spec[i]._arg [0] -> get_basename ( ); + if (spec._arg[0]) { + name = spec._arg[0]->get_basename(); } dxgsg9_cat.error() - << "NAME " << name << "\n" - << "MAT TYPE " - << _shader->_mat_spec[i]._piece - << " cgD3D9SetUniform failed " - << D3DERRORSTRING(hr); + << "NAME " << name << "\n" << "MAT TYPE " << spec._piece + << " cgD3D9SetUniform failed " << D3DERRORSTRING(hr); - CGerror error = cgGetError (); + CGerror error = cgGetError(); if (error != CG_NO_ERROR) { dxgsg9_cat.error() << " CG ERROR: " << cgGetErrorString(error) << "\n"; } @@ -713,25 +719,26 @@ disable_shader_texture_bindings(GSG *gsg) // reenable them. We may optimize this someday. //////////////////////////////////////////////////////////////////// void CLP(ShaderContext):: -update_shader_texture_bindings(CLP(ShaderContext) *prev, GSG *gsg) -{ - if (prev) prev->disable_shader_texture_bindings(gsg); +update_shader_texture_bindings(CLP(ShaderContext) *prev, GSG *gsg) { + if (prev) { + prev->disable_shader_texture_bindings(gsg); + } #ifdef HAVE_CG if (_cg_program) { - - for (int i=0; i<(int)_shader->_tex_spec.size(); i++) { - CGparameter p = _cg_parameter_map[_shader->_tex_spec[i]._id._seqno]; + for (size_t i = 0; i < _shader->_tex_spec.size(); ++i) { + const Shader::ShaderTexSpec &spec = _shader->_tex_spec[i]; + CGparameter p = _cg_parameter_map[spec._id._seqno]; if (p == NULL) { continue; } + Texture *tex = NULL; int view = gsg->get_current_tex_view_offset(); - InternalName *id = _shader->_tex_spec[i]._name; SamplerState sampler; - if (id != NULL) { - const ShaderInput *input = gsg->_target_shader->get_shader_input(id); + if (spec._name != NULL) { + const ShaderInput *input = gsg->_target_shader->get_shader_input(spec._name); tex = input->get_texture(); sampler = input->get_sampler(); @@ -741,31 +748,33 @@ update_shader_texture_bindings(CLP(ShaderContext) *prev, GSG *gsg) const TextureAttrib *texattrib = DCAST(TextureAttrib, gsg->_target_rs->get_attrib_def(TextureAttrib::get_class_slot())); nassertv(texattrib != (TextureAttrib *)NULL); - if (_shader->_tex_spec[i]._stage >= texattrib->get_num_on_stages()) { + if (spec._stage >= texattrib->get_num_on_stages()) { continue; } - TextureStage *stage = texattrib->get_on_stage(_shader->_tex_spec[i]._stage); + TextureStage *stage = texattrib->get_on_stage(spec._stage); tex = texattrib->get_on_texture(stage); sampler = texattrib->get_on_sampler(stage); view += stage->get_tex_view_offset(); } - if (_shader->_tex_spec[i]._suffix != 0) { - // The suffix feature is inefficient. It is a temporary hack. + + if (spec._suffix != 0) { + // The suffix feature is inefficient. It is a temporary hack. if (tex == 0) { continue; } - tex = tex->load_related(_shader->_tex_spec[i]._suffix); + tex = tex->load_related(spec._suffix); } - if ((tex == 0) || (tex->get_texture_type() != _shader->_tex_spec[i]._desired_type)) { + + if ((tex == 0) || (tex->get_texture_type() != spec._desired_type)) { continue; } + TextureContext *tc = tex->prepare_now(view, gsg->_prepared_objects, gsg); if (tc == (TextureContext*)NULL) { continue; } int texunit = cgGetParameterResourceIndex(p); - gsg->apply_texture(texunit, tc, sampler); } } diff --git a/panda/src/pgraph/shaderInput.I b/panda/src/pgraph/shaderInput.I index c7aed1de80..7761ec8a2f 100644 --- a/panda/src/pgraph/shaderInput.I +++ b/panda/src/pgraph/shaderInput.I @@ -688,9 +688,15 @@ get_ptr() const { //////////////////////////////////////////////////////////////////// INLINE const SamplerState &ShaderInput:: get_sampler() const { - return (_type == M_texture) - ? get_texture()->get_default_sampler() - : _sampler; + if (_type != M_texture) { + return _sampler; + + } else if (!_value.is_null()) { + return get_texture()->get_default_sampler(); + + } else { + return SamplerState::get_default(); + } } //////////////////////////////////////////////////////////////////// From e098da90302ba54e2a4dfeebd55fd5a67f456a02 Mon Sep 17 00:00:00 2001 From: rdb Date: Sun, 10 Jul 2016 15:48:17 +0200 Subject: [PATCH 4/4] Support uint8 indices in DX9 renderer --- doc/ReleaseNotes | 1 + panda/src/dxgsg9/dxGraphicsStateGuardian9.cxx | 2 + panda/src/dxgsg9/dxIndexBufferContext9.cxx | 53 +++++++++++-------- 3 files changed, 33 insertions(+), 23 deletions(-) diff --git a/doc/ReleaseNotes b/doc/ReleaseNotes index 7ffdb5ecc7..3796b840c0 100644 --- a/doc/ReleaseNotes +++ b/doc/ReleaseNotes @@ -24,6 +24,7 @@ This issue fixes several bugs that were still found in 1.9.2. * Fix assertion when using aux render targets in DX9 * Work around Cg bug generating invalid ASM for saturated tex loads * Fix issues with certain Cg shader inputs in DX9 +* Support uint8 index buffers in DX9 ------------------------ RELEASE 1.9.2 ------------------------ diff --git a/panda/src/dxgsg9/dxGraphicsStateGuardian9.cxx b/panda/src/dxgsg9/dxGraphicsStateGuardian9.cxx index c5d71e152d..cd4eb466e8 100644 --- a/panda/src/dxgsg9/dxGraphicsStateGuardian9.cxx +++ b/panda/src/dxgsg9/dxGraphicsStateGuardian9.cxx @@ -3584,6 +3584,8 @@ bind_light(Spotlight *light_obj, const NodePath &light, int light_id) { D3DFORMAT DXGraphicsStateGuardian9:: get_index_type(Geom::NumericType numeric_type) { switch (numeric_type) { + // NT_uint8 is automatically promoted to uint16. + case Geom::NT_uint8: case Geom::NT_uint16: return D3DFMT_INDEX16; diff --git a/panda/src/dxgsg9/dxIndexBufferContext9.cxx b/panda/src/dxgsg9/dxIndexBufferContext9.cxx index deaf5740bc..844a54cb42 100644 --- a/panda/src/dxgsg9/dxIndexBufferContext9.cxx +++ b/panda/src/dxgsg9/dxIndexBufferContext9.cxx @@ -30,9 +30,8 @@ TypeHandle DXIndexBufferContext9::_type_handle; DXIndexBufferContext9:: DXIndexBufferContext9(PreparedGraphicsObjects *pgo, GeomPrimitive *data) : IndexBufferContext(pgo, data), - _ibuffer(NULL) -{ - _managed = -1; + _ibuffer(NULL), + _managed(-1) { } //////////////////////////////////////////////////////////////////// @@ -42,8 +41,7 @@ DXIndexBufferContext9(PreparedGraphicsObjects *pgo, GeomPrimitive *data) : //////////////////////////////////////////////////////////////////// DXIndexBufferContext9:: ~DXIndexBufferContext9() { - - this -> free_ibuffer ( ); + free_ibuffer(); } //////////////////////////////////////////////////////////////////// @@ -82,13 +80,10 @@ free_ibuffer(void) { << "deleting index buffer " << _ibuffer << "\n"; } - if (DEBUG_INDEX_BUFFER) - { + if (DEBUG_INDEX_BUFFER) { RELEASE(_ibuffer, dxgsg9, "index buffer", RELEASE_ONCE); - } - else - { - _ibuffer -> Release ( ); + } else { + _ibuffer->Release(); } _ibuffer = NULL; @@ -113,6 +108,11 @@ allocate_ibuffer(DXScreenData &scrn, data_size = reader->get_data_size_bytes(); + if (reader->get_index_type() == GeomEnums::NT_uint8) { + // We widen 8-bits indices to 16-bits. + data_size *= 2; + } + _managed = scrn._managed_index_buffers; if (_managed) { @@ -164,16 +164,12 @@ create_ibuffer(DXScreenData &scrn, nassertv(reader->get_object() == get_data()); Thread *current_thread = reader->get_current_thread(); - this -> free_ibuffer ( ); + free_ibuffer(); PStatTimer timer(GraphicsStateGuardian::_create_index_buffer_pcollector, current_thread); - int data_size; - - data_size = reader->get_data_size_bytes(); - - this -> allocate_ibuffer(scrn, reader); + allocate_ibuffer(scrn, reader); } //////////////////////////////////////////////////////////////////// @@ -195,6 +191,11 @@ upload_data(const GeomPrimitivePipelineReader *reader, bool force) { } int data_size = reader->get_data_size_bytes(); + if (reader->get_index_type() == GeomEnums::NT_uint8) { + // We widen 8-bits indices to 16-bits. + data_size *= 2; + } + if (dxgsg9_cat.is_spam()) { dxgsg9_cat.spam() << "copying " << data_size @@ -206,12 +207,9 @@ upload_data(const GeomPrimitivePipelineReader *reader, bool force) { HRESULT hr; BYTE *local_pointer; - if (_managed) - { + if (_managed) { hr = _ibuffer->Lock(0, data_size, (void **) &local_pointer, 0); - } - else - { + } else { hr = _ibuffer->Lock(0, data_size, (void **) &local_pointer, D3DLOCK_DISCARD); } if (FAILED(hr)) { @@ -221,7 +219,16 @@ upload_data(const GeomPrimitivePipelineReader *reader, bool force) { } GraphicsStateGuardian::_data_transferred_pcollector.add_level(data_size); - memcpy(local_pointer, data_pointer, data_size); + + if (reader->get_index_type() == GeomEnums::NT_uint8) { + // Widen to 16-bits, as DirectX doesn't support 8-bits indices. + PN_uint16 *ptr = (PN_uint16 *)local_pointer; + for (size_t i = 0; i < data_size; i += 2) { + *ptr++ = (PN_uint16)*data_pointer++; + } + } else { + memcpy(local_pointer, data_pointer, data_size); + } _ibuffer->Unlock(); return true;