From 5d37d08cf8283324322a379e3470244acc3123bd Mon Sep 17 00:00:00 2001 From: Powei Feng Date: Tue, 1 Aug 2023 16:29:27 -0700 Subject: [PATCH 01/15] vulkan: fix TSAN in readpixels (#7023) --- filament/backend/src/vulkan/VulkanReadPixels.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/filament/backend/src/vulkan/VulkanReadPixels.cpp b/filament/backend/src/vulkan/VulkanReadPixels.cpp index 7940056964f..e299d206597 100644 --- a/filament/backend/src/vulkan/VulkanReadPixels.cpp +++ b/filament/backend/src/vulkan/VulkanReadPixels.cpp @@ -57,8 +57,8 @@ void TaskHandler::drain() { { std::unique_lock lock(syncPointMutex); done = true; + syncCondition.notify_one(); } - syncCondition.notify_one(); }); std::unique_lock lock(syncPointMutex); From 51c65ccfdcd39546503bc3323b19d702c509dcdd Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Tue, 1 Aug 2023 14:07:29 -0700 Subject: [PATCH 02/15] add picking to gltf_viewer for debugging --- .../android/filament/gltf/MainActivity.kt | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/android/samples/sample-gltf-viewer/src/main/java/com/google/android/filament/gltf/MainActivity.kt b/android/samples/sample-gltf-viewer/src/main/java/com/google/android/filament/gltf/MainActivity.kt index 06c9f141cd1..372cba57004 100644 --- a/android/samples/sample-gltf-viewer/src/main/java/com/google/android/filament/gltf/MainActivity.kt +++ b/android/samples/sample-gltf-viewer/src/main/java/com/google/android/filament/gltf/MainActivity.kt @@ -28,6 +28,7 @@ import com.google.android.filament.Fence import com.google.android.filament.IndirectLight import com.google.android.filament.Skybox import com.google.android.filament.View +import com.google.android.filament.View.OnPickCallback import com.google.android.filament.utils.* import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers @@ -56,7 +57,9 @@ class MainActivity : Activity() { private lateinit var modelViewer: ModelViewer private lateinit var titlebarHint: TextView private val doubleTapListener = DoubleTapListener() + private val singleTapListener = SingleTapListener() private lateinit var doubleTapDetector: GestureDetector + private lateinit var singleTapDetector: GestureDetector private var remoteServer: RemoteServer? = null private var statusToast: Toast? = null private var statusText: String? = null @@ -77,6 +80,7 @@ class MainActivity : Activity() { choreographer = Choreographer.getInstance() doubleTapDetector = GestureDetector(applicationContext, doubleTapListener) + singleTapDetector = GestureDetector(applicationContext, singleTapListener) modelViewer = ModelViewer(surfaceView) viewerContent.view = modelViewer.view @@ -88,6 +92,7 @@ class MainActivity : Activity() { surfaceView.setOnTouchListener { _, event -> modelViewer.onTouchEvent(event) doubleTapDetector.onTouchEvent(event) + singleTapDetector.onTouchEvent(event) true } @@ -229,6 +234,7 @@ class MainActivity : Activity() { modelViewer.scene.skybox = sky modelViewer.scene.indirectLight = ibl viewerContent.indirectLight = ibl + } } } @@ -430,4 +436,19 @@ class MainActivity : Activity() { return super.onDoubleTap(e) } } + + // Just for testing purposes + inner class SingleTapListener : GestureDetector.SimpleOnGestureListener() { + override fun onSingleTapUp(event: MotionEvent): Boolean { + modelViewer.view.pick( + event.x.toInt(), + surfaceView.height - event.y.toInt(), + surfaceView.handler, { + val name = modelViewer.asset!!.getName(it.renderable) + Log.v("Filament", "Picked ${it.renderable}: " + name) + }, + ) + return super.onSingleTapUp(event) + } + } } From adf3421f4ab8e300b85983237ff058b4e557c4b7 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Wed, 2 Aug 2023 12:39:52 -0700 Subject: [PATCH 03/15] Workaround Adreno bug causing picking to fail Adreno drivers don't support precision qualifiers in structs. fixes #6997 --- NEW_RELEASE_NOTES.md | 1 + shaders/src/common_instancing.glsl | 65 +++++++++++++++++------------- shaders/src/depth_main.fs | 10 ++--- shaders/src/getters.fs | 2 +- shaders/src/getters.vs | 14 +++---- shaders/src/light_directional.fs | 4 +- shaders/src/light_punctual.fs | 4 +- shaders/src/main.fs | 2 +- shaders/src/main.vs | 10 ++--- shaders/src/shading_unlit.fs | 2 +- 10 files changed, 61 insertions(+), 53 deletions(-) diff --git a/NEW_RELEASE_NOTES.md b/NEW_RELEASE_NOTES.md index 4a1a9c7fa7e..72fdf587ac9 100644 --- a/NEW_RELEASE_NOTES.md +++ b/NEW_RELEASE_NOTES.md @@ -7,3 +7,4 @@ for next branch cut* header. appropriate header in [RELEASE_NOTES.md](./RELEASE_NOTES.md). ## Release notes for next branch cut +- backend: fix #6997 : picking can fail on Adreno [⚠️ **New Material Version**] diff --git a/shaders/src/common_instancing.glsl b/shaders/src/common_instancing.glsl index db68b86ca5e..2bc74923966 100644 --- a/shaders/src/common_instancing.glsl +++ b/shaders/src/common_instancing.glsl @@ -2,41 +2,48 @@ // Instancing // ------------------------------------------------------------------------------------------------ -PerRenderableData object_uniforms; +highp mat4 object_uniforms_worldFromModelMatrix; +highp mat3 object_uniforms_worldFromModelNormalMatrix; +highp int object_uniforms_morphTargetCount; +highp int object_uniforms_flagsChannels; // see packFlags() below (0x00000fll) +highp int object_uniforms_objectId; // used for picking +highp float object_uniforms_userData; // TODO: We need a better solution, this currently holds the average local scale for the renderable + +void initObjectUniforms() { + // Adreno drivers workarounds: + // - We need to copy each field separately because non-const array access in a UBO fails + // e.g.: this fails `p = objectUniforms.data[instance_index];` + // - We can't use a struct to hold the result because Adreno driver ignore precision qualifiers + // on fields of structs, unless they're in a UBO (which we just copied out of). -void initInstanceUniforms(out PerRenderableData p) { #if defined(FILAMENT_HAS_FEATURE_INSTANCING) - // We're copying each field separately to workaround an issue in some Adreno drivers - // that fail on non-const array access in a UBO. Accessing the fields works however. - // e.g.: this fails `p = objectUniforms.data[instance_index];` - p.worldFromModelMatrix = objectUniforms.data[instance_index].worldFromModelMatrix; - p.worldFromModelNormalMatrix = objectUniforms.data[instance_index].worldFromModelNormalMatrix; - p.morphTargetCount = objectUniforms.data[instance_index].morphTargetCount; - p.flagsChannels = objectUniforms.data[instance_index].flagsChannels; - p.objectId = objectUniforms.data[instance_index].objectId; - p.userData = objectUniforms.data[instance_index].userData; -#else - p.worldFromModelMatrix = objectUniforms.data[0].worldFromModelMatrix; - p.worldFromModelNormalMatrix = objectUniforms.data[0].worldFromModelNormalMatrix; - p.morphTargetCount = objectUniforms.data[0].morphTargetCount; - p.flagsChannels = objectUniforms.data[0].flagsChannels; - p.objectId = objectUniforms.data[0].objectId; - p.userData = objectUniforms.data[0].userData; +#if defined(MATERIAL_HAS_INSTANCES) + if ((objectUniforms.data[0].flagsChannels & FILAMENT_OBJECT_INSTANCE_BUFFER_BIT) == 0) { + // the material manages instancing, all instances share the same uniform block. + object_uniforms_worldFromModelMatrix = objectUniforms.data[0].worldFromModelMatrix; + object_uniforms_worldFromModelNormalMatrix = objectUniforms.data[0].worldFromModelNormalMatrix; + object_uniforms_morphTargetCount = objectUniforms.data[0].morphTargetCount; + object_uniforms_flagsChannels = objectUniforms.data[0].flagsChannels; + object_uniforms_objectId = objectUniforms.data[0].objectId; + object_uniforms_userData = objectUniforms.data[0].userData; + } else #endif -} - -void initObjectUniforms(out PerRenderableData p) { -#if defined(FILAMENT_HAS_FEATURE_INSTANCING) && defined(MATERIAL_HAS_INSTANCES) - if ((objectUniforms.data[0].flagsChannels & FILAMENT_OBJECT_INSTANCE_BUFFER_BIT) != 0) { + { // the object has an instance buffer - initInstanceUniforms(p); - return; + object_uniforms_worldFromModelMatrix = objectUniforms.data[instance_index].worldFromModelMatrix; + object_uniforms_worldFromModelNormalMatrix = objectUniforms.data[instance_index].worldFromModelNormalMatrix; + object_uniforms_morphTargetCount = objectUniforms.data[instance_index].morphTargetCount; + object_uniforms_flagsChannels = objectUniforms.data[instance_index].flagsChannels; + object_uniforms_objectId = objectUniforms.data[instance_index].objectId; + object_uniforms_userData = objectUniforms.data[instance_index].userData; } - // the material manages instancing, all instances share the same uniform block. - p = objectUniforms.data[0]; #else - // automatic instancing was used, each instance has its own uniform block. - initInstanceUniforms(p); + object_uniforms_worldFromModelMatrix = objectUniforms.data[0].worldFromModelMatrix; + object_uniforms_worldFromModelNormalMatrix = objectUniforms.data[0].worldFromModelNormalMatrix; + object_uniforms_morphTargetCount = objectUniforms.data[0].morphTargetCount; + object_uniforms_flagsChannels = objectUniforms.data[0].flagsChannels; + object_uniforms_objectId = objectUniforms.data[0].objectId; + object_uniforms_userData = objectUniforms.data[0].userData; #endif } diff --git a/shaders/src/depth_main.fs b/shaders/src/depth_main.fs index 0f09d59cdee..e0cb5f690c4 100644 --- a/shaders/src/depth_main.fs +++ b/shaders/src/depth_main.fs @@ -25,7 +25,7 @@ highp vec2 computeDepthMomentsVSM(const highp float depth); void main() { filament_lodBias = frameUniforms.lodBias; - initObjectUniforms(object_uniforms); + initObjectUniforms(); #if defined(BLEND_MODE_MASKED) || ((defined(BLEND_MODE_TRANSPARENT) || defined(BLEND_MODE_FADE)) && defined(MATERIAL_HAS_TRANSPARENT_SHADOW)) MaterialInputs inputs; @@ -58,12 +58,12 @@ void main() { fragColor.zw = computeDepthMomentsVSM(-1.0 / depth); // requires at least RGBA16F #elif defined(VARIANT_HAS_PICKING) #if MATERIAL_FEATURE_LEVEL == 0 - outPicking.a = float((object_uniforms.objectId / 65536) % 256) / 255.0; - outPicking.b = float((object_uniforms.objectId / 256) % 256) / 255.0; - outPicking.g = float( object_uniforms.objectId % 256) / 255.0; + outPicking.a = float((object_uniforms_objectId / 65536) % 256) / 255.0; + outPicking.b = float((object_uniforms_objectId / 256) % 256) / 255.0; + outPicking.g = float( object_uniforms_objectId % 256) / 255.0; outPicking.r = vertex_position.z / vertex_position.w; #else - outPicking.x = float(object_uniforms.objectId); + outPicking.x = float(object_uniforms_objectId); outPicking.y = vertex_position.z / vertex_position.w; #endif #if __VERSION__ == 100 diff --git a/shaders/src/getters.fs b/shaders/src/getters.fs index d2036de7845..46e6cf4e346 100644 --- a/shaders/src/getters.fs +++ b/shaders/src/getters.fs @@ -4,7 +4,7 @@ /** sort-of public */ float getObjectUserData() { - return object_uniforms.userData; + return object_uniforms_userData; } //------------------------------------------------------------------------------ diff --git a/shaders/src/getters.vs b/shaders/src/getters.vs index 32da5b75719..cc06a19f5c3 100644 --- a/shaders/src/getters.vs +++ b/shaders/src/getters.vs @@ -4,17 +4,17 @@ /** @public-api */ mat4 getWorldFromModelMatrix() { - return object_uniforms.worldFromModelMatrix; + return object_uniforms_worldFromModelMatrix; } /** @public-api */ mat3 getWorldFromModelNormalMatrix() { - return object_uniforms.worldFromModelNormalMatrix; + return object_uniforms_worldFromModelNormalMatrix; } /** sort-of public */ float getObjectUserData() { - return object_uniforms.userData; + return object_uniforms_userData; } //------------------------------------------------------------------------------ @@ -80,7 +80,7 @@ void skinPosition(inout vec3 p, const uvec4 ids, const vec4 weights) { void morphPosition(inout vec4 p) { ivec3 texcoord = ivec3(getVertexIndex() % MAX_MORPH_TARGET_BUFFER_WIDTH, getVertexIndex() / MAX_MORPH_TARGET_BUFFER_WIDTH, 0); - int c = object_uniforms.morphTargetCount; + int c = object_uniforms_morphTargetCount; for (int i = 0; i < c; ++i) { float w = morphingUniforms.weights[i][0]; if (w != 0.0) { @@ -93,7 +93,7 @@ void morphPosition(inout vec4 p) { void morphNormal(inout vec3 n) { vec3 baseNormal = n; ivec3 texcoord = ivec3(getVertexIndex() % MAX_MORPH_TARGET_BUFFER_WIDTH, getVertexIndex() / MAX_MORPH_TARGET_BUFFER_WIDTH, 0); - int c = object_uniforms.morphTargetCount; + int c = object_uniforms_morphTargetCount; for (int i = 0; i < c; ++i) { float w = morphingUniforms.weights[i][0]; if (w != 0.0) { @@ -113,7 +113,7 @@ vec4 getPosition() { #if defined(VARIANT_HAS_SKINNING_OR_MORPHING) - if ((object_uniforms.flagsChannels & FILAMENT_OBJECT_MORPHING_ENABLED_BIT) != 0) { + if ((object_uniforms_flagsChannels & FILAMENT_OBJECT_MORPHING_ENABLED_BIT) != 0) { #if defined(LEGACY_MORPHING) pos += morphingUniforms.weights[0] * mesh_custom0; pos += morphingUniforms.weights[1] * mesh_custom1; @@ -124,7 +124,7 @@ vec4 getPosition() { #endif } - if ((object_uniforms.flagsChannels & FILAMENT_OBJECT_SKINNING_ENABLED_BIT) != 0) { + if ((object_uniforms_flagsChannels & FILAMENT_OBJECT_SKINNING_ENABLED_BIT) != 0) { skinPosition(pos.xyz, mesh_bone_indices, mesh_bone_weights); } diff --git a/shaders/src/light_directional.fs b/shaders/src/light_directional.fs index c2017c66807..4a4ad944cc9 100644 --- a/shaders/src/light_directional.fs +++ b/shaders/src/light_directional.fs @@ -36,7 +36,7 @@ void evaluateDirectionalLight(const MaterialInputs material, Light light = getDirectionalLight(); - int channels = object_uniforms.flagsChannels & 0xFF; + int channels = object_uniforms_flagsChannels & 0xFF; if ((light.channels & channels) == 0) { return; } @@ -60,7 +60,7 @@ void evaluateDirectionalLight(const MaterialInputs material, visibility = shadow(true, light_shadowMap, cascade, shadowPosition, 0.0); } if ((frameUniforms.directionalShadows & 0x2) != 0 && visibility > 0.0) { - if ((object_uniforms.flagsChannels & FILAMENT_OBJECT_CONTACT_SHADOWS_BIT) != 0) { + if ((object_uniforms_flagsChannels & FILAMENT_OBJECT_CONTACT_SHADOWS_BIT) != 0) { ssContactShadowOcclusion = screenSpaceContactShadow(light.l); } } diff --git a/shaders/src/light_punctual.fs b/shaders/src/light_punctual.fs index 3de51613585..1c465a7f8f1 100644 --- a/shaders/src/light_punctual.fs +++ b/shaders/src/light_punctual.fs @@ -191,7 +191,7 @@ void evaluatePunctualLights(const MaterialInputs material, uint index = froxel.recordOffset; uint end = index + froxel.count; - int channels = object_uniforms.flagsChannels & 0xFF; + int channels = object_uniforms_flagsChannels & 0xFF; // Iterate point lights for ( ; index < end; index++) { @@ -225,7 +225,7 @@ void evaluatePunctualLights(const MaterialInputs material, shadowPosition, light.zLight); } if (light.contactShadows && visibility > 0.0) { - if ((object_uniforms.flagsChannels & FILAMENT_OBJECT_CONTACT_SHADOWS_BIT) != 0) { + if ((object_uniforms_flagsChannels & FILAMENT_OBJECT_CONTACT_SHADOWS_BIT) != 0) { visibility *= 1.0 - screenSpaceContactShadow(light.l); } } diff --git a/shaders/src/main.fs b/shaders/src/main.fs index 047fd265e77..06265a2f50b 100644 --- a/shaders/src/main.fs +++ b/shaders/src/main.fs @@ -23,7 +23,7 @@ void blendPostLightingColor(const MaterialInputs material, inout vec4 color) { void main() { filament_lodBias = frameUniforms.lodBias; - initObjectUniforms(object_uniforms); + initObjectUniforms(); // See shading_parameters.fs // Computes global variables we need to evaluate material and lighting diff --git a/shaders/src/main.vs b/shaders/src/main.vs index 9debbcd63d3..de940108cf7 100644 --- a/shaders/src/main.vs +++ b/shaders/src/main.vs @@ -15,7 +15,7 @@ void main() { # endif #endif - initObjectUniforms(object_uniforms); + initObjectUniforms(); // Initialize the inputs to sensible default values, see material_inputs.vs #if defined(USE_OPTIMIZED_DEPTH_VERTEX_SHADER) @@ -45,7 +45,7 @@ void main() { toTangentFrame(mesh_tangents, material.worldNormal, vertex_worldTangent.xyz); #if defined(VARIANT_HAS_SKINNING_OR_MORPHING) - if ((object_uniforms.flagsChannels & FILAMENT_OBJECT_MORPHING_ENABLED_BIT) != 0) { + if ((object_uniforms_flagsChannels & FILAMENT_OBJECT_MORPHING_ENABLED_BIT) != 0) { #if defined(LEGACY_MORPHING) vec3 normal0, normal1, normal2, normal3; toTangentFrame(mesh_custom4, normal0); @@ -63,7 +63,7 @@ void main() { #endif } - if ((object_uniforms.flagsChannels & FILAMENT_OBJECT_SKINNING_ENABLED_BIT) != 0) { + if ((object_uniforms_flagsChannels & FILAMENT_OBJECT_SKINNING_ENABLED_BIT) != 0) { skinNormal(material.worldNormal, mesh_bone_indices, mesh_bone_weights); skinNormal(vertex_worldTangent.xyz, mesh_bone_indices, mesh_bone_weights); } @@ -81,7 +81,7 @@ void main() { toTangentFrame(mesh_tangents, material.worldNormal); #if defined(VARIANT_HAS_SKINNING_OR_MORPHING) - if ((object_uniforms.flagsChannels & FILAMENT_OBJECT_MORPHING_ENABLED_BIT) != 0) { + if ((object_uniforms_flagsChannels & FILAMENT_OBJECT_MORPHING_ENABLED_BIT) != 0) { #if defined(LEGACY_MORPHING) vec3 normal0, normal1, normal2, normal3; toTangentFrame(mesh_custom4, normal0); @@ -99,7 +99,7 @@ void main() { #endif } - if ((object_uniforms.flagsChannels & FILAMENT_OBJECT_SKINNING_ENABLED_BIT) != 0) { + if ((object_uniforms_flagsChannels & FILAMENT_OBJECT_SKINNING_ENABLED_BIT) != 0) { skinNormal(material.worldNormal, mesh_bone_indices, mesh_bone_weights); } #endif diff --git a/shaders/src/shading_unlit.fs b/shaders/src/shading_unlit.fs index a393c151235..af30ddd7db0 100644 --- a/shaders/src/shading_unlit.fs +++ b/shaders/src/shading_unlit.fs @@ -54,7 +54,7 @@ vec4 evaluateMaterial(const MaterialInputs material) { visibility = shadow(true, light_shadowMap, cascade, shadowPosition, 0.0); } if ((frameUniforms.directionalShadows & 0x2) != 0 && visibility > 0.0) { - if ((object_uniforms.flagsChannels & FILAMENT_OBJECT_CONTACT_SHADOWS_BIT) != 0) { + if ((object_uniforms_flagsChannels & FILAMENT_OBJECT_CONTACT_SHADOWS_BIT) != 0) { visibility *= (1.0 - screenSpaceContactShadow(frameUniforms.lightDirection)); } } From 80014bf2b1085b911e0df99135873333010e89ae Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Wed, 2 Aug 2023 15:32:49 -0700 Subject: [PATCH 04/15] fix a possible overflow when picking We don't need to convert the object id to float, instead we can just "reinterpret_cast" it. With the current possible values of Entities, there was a risk of overflow once the age gets to 128 (very rare). --- filament/src/details/View.cpp | 5 ++--- shaders/src/depth_main.fs | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/filament/src/details/View.cpp b/filament/src/details/View.cpp index 3d05edf229a..4d5778145e4 100644 --- a/filament/src/details/View.cpp +++ b/filament/src/details/View.cpp @@ -1009,9 +1009,8 @@ void FView::executePickingQueries(backend::DriverApi& driver, &pQuery->result.renderable, 4u * 4u, // 4*float backend::PixelDataFormat::RG, backend::PixelDataType::FLOAT, pQuery->handler, [](void*, size_t, void* user) { - FPickingQuery* pQuery = static_cast(user); - float const identity = *((float*)((char*)&pQuery->result.renderable)); - pQuery->result.renderable = Entity::import(int32_t(identity)); + FPickingQuery* const pQuery = static_cast(user); + // pQuery->result.renderable already contains the right value! pQuery->result.fragCoords = { pQuery->x, pQuery->y, float(1.0 - pQuery->result.depth) }; pQuery->callback(pQuery->result, pQuery); diff --git a/shaders/src/depth_main.fs b/shaders/src/depth_main.fs index e0cb5f690c4..26641c4c1f3 100644 --- a/shaders/src/depth_main.fs +++ b/shaders/src/depth_main.fs @@ -63,7 +63,7 @@ void main() { outPicking.g = float( object_uniforms_objectId % 256) / 255.0; outPicking.r = vertex_position.z / vertex_position.w; #else - outPicking.x = float(object_uniforms_objectId); + outPicking.x = intBitsToFloat(object_uniforms_objectId); outPicking.y = vertex_position.z / vertex_position.w; #endif #if __VERSION__ == 100 From 7d01e0349b341a987dfbbe3725ac189240091e13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EC=A1=B0=EB=8B=A4=EB=8B=88=EC=97=98=28Daniel=20Cho=29?= Date: Fri, 7 Jul 2023 17:54:04 +0900 Subject: [PATCH 05/15] Fix rendering issue when using DoF --- filament/src/materials/dof/dof.mat | 2 ++ 1 file changed, 2 insertions(+) diff --git a/filament/src/materials/dof/dof.mat b/filament/src/materials/dof/dof.mat index cdf59458cda..b6bb5b1ea78 100644 --- a/filament/src/materials/dof/dof.mat +++ b/filament/src/materials/dof/dof.mat @@ -477,6 +477,7 @@ void fastTile(inout vec4 color, inout float alpha, } alpha = cocToAlpha(kernelSize); color *= rcp(sampleCount(ringCountFast)); + color = min(vec4(MEDIUMP_FLT_MAX), color); } void foregroundTile(inout vec4 foreground, inout float fgOpacity, @@ -658,6 +659,7 @@ void postProcess(inout PostProcessInputs postProcess) { // the downside of doing this is that we couldn't use a different upscaler for each // layer, but this is a lot less costly postProcess.color = foreground + (1.0 - fgOpacity) * background; + postProcess.color = min(vec4(MEDIUMP_FLT_MAX), postProcess.color); postProcess.alpha = fgOpacity + (1.0 - fgOpacity) * bgOpacity; } From 74b64a54518652e415acf3270ed56b4936c79b49 Mon Sep 17 00:00:00 2001 From: Romain Guy Date: Thu, 3 Aug 2023 15:03:03 -0700 Subject: [PATCH 06/15] Update README.md (#7037) --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index f1962fa0748..99b310357df 100644 --- a/README.md +++ b/README.md @@ -40,6 +40,7 @@ Here are all the libraries available in the group `com.google.android.filament`: | Artifact | Description | | ------------- | ------------- | | [![filament-android](https://maven-badges.herokuapp.com/maven-central/com.google.android.filament/filament-android/badge.svg?subject=filament-android)](https://maven-badges.herokuapp.com/maven-central/com.google.android.filament/filament-android) | The Filament rendering engine itself. | +| [![filament-android-debug](https://maven-badges.herokuapp.com/maven-central/com.google.android.filament/filament-android-debug/badge.svg?subject=filament-android-debug)](https://maven-badges.herokuapp.com/maven-central/com.google.android.filament/filament-android-debug) | Debug version of `filament-android`. | | [![gltfio-android](https://maven-badges.herokuapp.com/maven-central/com.google.android.filament/gltfio-android/badge.svg?subject=gltfio-android)](https://maven-badges.herokuapp.com/maven-central/com.google.android.filament/gltfio-android) | A glTF 2.0 loader for Filament, depends on `filament-android`. | | [![filament-utils-android](https://maven-badges.herokuapp.com/maven-central/com.google.android.filament/filament-utils-android/badge.svg?subject=filament-utils-android)](https://maven-badges.herokuapp.com/maven-central/com.google.android.filament/filament-utils-android) | KTX loading, Kotlin math, and camera utilities, depends on `gltfio-android`. | | [![filamat-android](https://maven-badges.herokuapp.com/maven-central/com.google.android.filament/filamat-android/badge.svg?subject=filamat-android)](https://maven-badges.herokuapp.com/maven-central/com.google.android.filament/filamat-android) | A runtime material builder/compiler. This library is large but contains a full shader compiler/validator/optimizer and supports both OpenGL and Vulkan. | From 2a12f71f962a729cf001c79e2ef170313d36da8f Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Thu, 3 Aug 2023 12:18:22 -0700 Subject: [PATCH 07/15] fix a crash when shutting the engine down We were breaking the promise of pending shader compilation jobs by destroying the corresponding std::promise embedded in the job queue. In practice there was no danger of a deadlock by construction, but std::promise throws an exception in that case. On builds without exception enabled, this we be turned into an abort(). We fix this by using our own mechanism for signaling instead of std::promise. This ends up be more lightweight anyways. Fix: #6933 --- .../src/opengl/ShaderCompilerService.cpp | 74 +++++++++++++------ 1 file changed, 50 insertions(+), 24 deletions(-) diff --git a/filament/backend/src/opengl/ShaderCompilerService.cpp b/filament/backend/src/opengl/ShaderCompilerService.cpp index 59b85d134db..b2a5863562f 100644 --- a/filament/backend/src/opengl/ShaderCompilerService.cpp +++ b/filament/backend/src/opengl/ShaderCompilerService.cpp @@ -32,7 +32,6 @@ #include #include -#include #include #include #include @@ -85,9 +84,40 @@ struct ShaderCompilerService::ProgramToken { GLuint program = 0; } gl; // 12 bytes + + // Sets the programBinary, typically from the compiler thread, and signal the main thread. + // This is similar to std::promise::set_value. + void set(ProgramBinary programBinary) noexcept { + using std::swap; + std::unique_lock const l(lock); + swap(binary, programBinary); + signaled = true; + cond.notify_one(); + } + + // Get the programBinary, wait if necessary. + // This is similar to std::future::get + ProgramBinary const& get() const noexcept { + std::unique_lock l(lock); + cond.wait(l, [this](){ return signaled; }); + return binary; + } + + // Checks if the programBinary is ready. + // This is similar to std::future::wait_for(0s) + bool isReady() const noexcept { + std::unique_lock l(lock); + using namespace std::chrono_literals; + return cond.wait_for(l, 0s, [this](){ return signaled; }); + } + BlobCacheKey key; - std::future binary; - bool canceled = false; + mutable utils::Mutex lock; + mutable utils::Condition cond; + ProgramBinary binary; + bool signaled = false; + + bool canceled = false; // not part of the signaling }; void ShaderCompilerService::setUserData(const program_token_t& token, void* user) noexcept { @@ -242,16 +272,22 @@ void ShaderCompilerService::init() noexcept { } void ShaderCompilerService::terminate() noexcept { - mCompilerThreadPool.terminate(); - - // We could have some pending callbacks here, we need to execute them + // We could have some pending callbacks here, we need to execute them. + // This is equivalent to calling cancelTickOp() on all active tokens. for (auto&& op: mRunAtNextTickOps) { - Job const& job = std::get<2>(op); - if (job.callback) { + auto const& [priority, token, job] = op; + if (!token && job.callback) { + // This is a little fragile here. We know by construction that jobs that have a + // null token are the ones that dispatch the user callbacks. mDriver.scheduleCallback(job.handler, job.user, job.callback); } } mRunAtNextTickOps.clear(); + + // Finally stop the thread pool immediately. Pending jobs will be discarded. We guarantee by + // construction that nobody is waiting on a token (because waiting is only done on the main + // backend thread, and if we're here, we're on the backend main thread). + mCompilerThreadPool.terminate(); } ShaderCompilerService::program_token_t ShaderCompilerService::createProgram( @@ -268,13 +304,9 @@ ShaderCompilerService::program_token_t ShaderCompilerService::createProgram( if (!token->gl.program) { CompilerPriorityQueue const priorityQueue = program.getPriorityQueue(); if (mShaderCompilerThreadCount) { - // set the future in the token and pass the promise to the worker thread - std::promise promise; - token->binary = promise.get_future(); // queue a compile job mCompilerThreadPool.queue(priorityQueue, token, - [this, &gl, promise = std::move(promise), - program = std::move(program), token]() mutable { + [this, &gl, program = std::move(program), token]() mutable { // compile the shaders std::array shaders{}; @@ -326,7 +358,7 @@ ShaderCompilerService::program_token_t ShaderCompilerService::createProgram( #endif // we don't need to check for success here, it'll be done on the // main thread side. - promise.set_value(binary); + token->set(std::move(binary)); }); } else { @@ -346,10 +378,8 @@ ShaderCompilerService::program_token_t ShaderCompilerService::createProgram( // TODO: see if we could completely eliminate this callback here // and instead just rely on token->gl.program being atomically // set by the compiler thread. - assert_invariant(token->binary.valid()); // we're using the compiler thread, check if the program is ready, no-op if not. - using namespace std::chrono_literals; - if (token->binary.wait_for(0s) != std::future_status::ready) { + if (!token->isReady()) { return false; } // program binary is ready, retrieve it without blocking @@ -453,7 +483,7 @@ GLuint ShaderCompilerService::getProgram(ShaderCompilerService::program_token_t& glDeleteProgram(token->gl.program); } - token = nullptr; + token.reset(); } void ShaderCompilerService::tick() { @@ -512,7 +542,7 @@ void ShaderCompilerService::notifyWhenAllProgramsAreReady(CompilerPriorityQueue // ------------------------------------------------------------------------------------------------ void ShaderCompilerService::getProgramFromCompilerPool(program_token_t& token) noexcept { - ProgramToken::ProgramBinary const binary{ token->binary.get() }; + ProgramToken::ProgramBinary const& binary{ token->get() }; if (!token->canceled) { token->gl.shaders = binary.shaders; if (UTILS_LIKELY(mUseSharedContext)) { @@ -532,9 +562,6 @@ GLuint ShaderCompilerService::initialize(program_token_t& token) noexcept { SYSTRACE_CALL(); if (!token->gl.program) { if (mShaderCompilerThreadCount) { - // Block until the program is ready. This could take a very long time. - assert_invariant(token->binary.valid()); - // we need this program right now, so move it to the head of the queue. mCompilerThreadPool.makeUrgent(token); @@ -803,8 +830,7 @@ void ShaderCompilerService::runAtNextTick(CompilerPriorityQueue priority, void ShaderCompilerService::cancelTickOp(program_token_t token) noexcept { // We do a linear search here, but this is rare, and we know the list is pretty small. auto& ops = mRunAtNextTickOps; - auto pos = std::find_if(ops.begin(), ops.end(), - [&](const auto& item) { + auto pos = std::find_if(ops.begin(), ops.end(), [&](const auto& item) { return std::get<1>(item) == token; }); if (pos != ops.end()) { From f68825f2ed00468f0d312f668234bc34095238e4 Mon Sep 17 00:00:00 2001 From: Powei Feng Date: Fri, 4 Aug 2023 11:13:57 -0700 Subject: [PATCH 08/15] vulkan: fix fence initialization (#7038) Previously, we have a VulkanSync with a default constructor that allows us to have sync objects that returns error when actual fences are not yet present. We need to replicate that with VulkanFence since sync objects have been removed from the API. Fixes #7034 --- filament/backend/src/vulkan/VulkanDriver.cpp | 10 ++++++++-- filament/backend/src/vulkan/VulkanHandles.h | 4 +++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/filament/backend/src/vulkan/VulkanDriver.cpp b/filament/backend/src/vulkan/VulkanDriver.cpp index 30573ddf194..14215434975 100644 --- a/filament/backend/src/vulkan/VulkanDriver.cpp +++ b/filament/backend/src/vulkan/VulkanDriver.cpp @@ -503,7 +503,7 @@ void VulkanDriver::destroyRenderTarget(Handle rth) { void VulkanDriver::createFenceR(Handle fh, int) { VulkanCommandBuffer const& commandBuffer = mCommands->get(); - construct(fh, commandBuffer); + construct(fh, commandBuffer.fence); } void VulkanDriver::createSwapChainR(Handle sch, void* nativeWindow, uint64_t flags) { @@ -579,7 +579,7 @@ Handle VulkanDriver::createRenderTargetS() noexcept { } Handle VulkanDriver::createFenceS() noexcept { - return allocHandle(); + return initHandle(); } Handle VulkanDriver::createSwapChainS() noexcept { @@ -664,6 +664,12 @@ void VulkanDriver::destroyFence(Handle fh) { FenceStatus VulkanDriver::wait(Handle fh, uint64_t timeout) { auto& cmdfence = handle_cast(fh)->fence; + if (!cmdfence) { + // If wait is called before a fence actually exists, we return timeout. This matches the + // current behavior in OpenGLDriver, but we should eventually reconsider a different error + // code. + return FenceStatus::TIMEOUT_EXPIRED; + } // Internally we use the VK_INCOMPLETE status to mean "not yet submitted". // When this fence gets submitted, its status changes to VK_NOT_READY. diff --git a/filament/backend/src/vulkan/VulkanHandles.h b/filament/backend/src/vulkan/VulkanHandles.h index fb82e78f738..fe5fc2227e0 100644 --- a/filament/backend/src/vulkan/VulkanHandles.h +++ b/filament/backend/src/vulkan/VulkanHandles.h @@ -129,7 +129,9 @@ struct VulkanRenderPrimitive : public HwRenderPrimitive { }; struct VulkanFence : public HwFence { - explicit VulkanFence(const VulkanCommandBuffer& commands) : fence(commands.fence) {} + VulkanFence() = default; + explicit VulkanFence(std::shared_ptr fence) : fence(fence) {} + std::shared_ptr fence; }; From 2468a3a8541325296a54512cb8fcc15a09463f4c Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Fri, 4 Aug 2023 15:41:43 -0700 Subject: [PATCH 09/15] fix a typo causing EXT_color_buffer_float enabled on al ES3 devices b/287126679 --- filament/backend/src/opengl/OpenGLContext.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/filament/backend/src/opengl/OpenGLContext.cpp b/filament/backend/src/opengl/OpenGLContext.cpp index 14d7c679c7f..5989941d57d 100644 --- a/filament/backend/src/opengl/OpenGLContext.cpp +++ b/filament/backend/src/opengl/OpenGLContext.cpp @@ -502,7 +502,7 @@ void OpenGLContext::initExtensionsGLES() noexcept { // ES 3.x implies EXT_discard_framebuffer and OES_vertex_array_object if (state.major >= 3) { - ext.EXT_color_buffer_float = true; + ext.EXT_discard_framebuffer = true; ext.OES_vertex_array_object = true; } } From 96cccc83c6cd4c87ea658a09cac331b7439405ab Mon Sep 17 00:00:00 2001 From: Romain Guy Date: Sun, 6 Aug 2023 08:44:55 -0700 Subject: [PATCH 10/15] Update BUILDING.md --- BUILDING.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/BUILDING.md b/BUILDING.md index d2191109cfd..9c0f4d08c95 100644 --- a/BUILDING.md +++ b/BUILDING.md @@ -5,7 +5,7 @@ To build Filament, you must first install the following tools: - CMake 3.19 (or more recent) -- clang 7.0 (or more recent) +- clang 14.0 (or more recent) - [ninja 1.10](https://github.com/ninja-build/ninja/wiki/Pre-built-Ninja-packages) (or more recent) Additional dependencies may be required for your operating system. Please refer to the appropriate @@ -87,10 +87,10 @@ Options can also be set with the CMake GUI. Make sure you've installed the following dependencies: -- `clang-7` or higher +- `clang-14` or higher - `libglu1-mesa-dev` -- `libc++-7-dev` (`libcxx-devel` and `libcxx-static` on Fedora) or higher -- `libc++abi-7-dev` (`libcxxabi-static` on Fedora) or higher +- `libc++-14-dev` (`libcxx-devel` and `libcxx-static` on Fedora) or higher +- `libc++abi-14-dev` (`libcxxabi-static` on Fedora) or higher - `ninja-build` - `libxi-dev` - `libxcomposite-dev` (`libXcomposite-devel` on Fedora) @@ -114,7 +114,7 @@ Your Linux distribution might default to `gcc` instead of `clang`, if that's the ``` $ mkdir out/cmake-release $ cd out/cmake-release -# Or use a specific version of clang, for instance /usr/bin/clang-7 +# Or use a specific version of clang, for instance /usr/bin/clang-14 $ CC=/usr/bin/clang CXX=/usr/bin/clang++ CXXFLAGS=-stdlib=libc++ \ cmake -G Ninja -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=../release/filament ../.. ``` @@ -124,8 +124,8 @@ solution is to use `update-alternatives` to both change the default compiler, an specific version of clang: ``` -$ update-alternatives --install /usr/bin/clang clang /usr/bin/clang-7 100 -$ update-alternatives --install /usr/bin/clang++ clang++ /usr/bin/clang++-7 100 +$ update-alternatives --install /usr/bin/clang clang /usr/bin/clang-14 100 +$ update-alternatives --install /usr/bin/clang++ clang++ /usr/bin/clang++-14 100 $ update-alternatives --install /usr/bin/cc cc /usr/bin/clang 100 $ update-alternatives --install /usr/bin/c++ c++ /usr/bin/clang++ 100 ``` From 6b6827b70d4c9406c44285c3a4f9c9d4bab5ec35 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Tue, 8 Aug 2023 08:59:06 -0700 Subject: [PATCH 11/15] add a GLES compiler unit test (#7050) * add a GLES compiler unit test * Update filament/test/compiler_test.cpp Co-authored-by: Ben Doherty --------- Co-authored-by: Ben Doherty --- filament/test/CMakeLists.txt | 8 ++ filament/test/compiler_test.cpp | 136 ++++++++++++++++++++++++++++++++ 2 files changed, 144 insertions(+) create mode 100644 filament/test/compiler_test.cpp diff --git a/filament/test/CMakeLists.txt b/filament/test/CMakeLists.txt index f9aedc5468d..7b498008677 100644 --- a/filament/test/CMakeLists.txt +++ b/filament/test/CMakeLists.txt @@ -56,6 +56,14 @@ if (TNT_DEV) target_link_libraries(test_depth PRIVATE utils) endif() +if (ANDROID) + add_executable(test_compiler compiler_test.cpp) + target_link_libraries(test_compiler PRIVATE gtest) + target_link_libraries(test_compiler PRIVATE utils) + target_link_libraries(test_compiler PRIVATE EGL) + target_link_libraries(test_compiler PRIVATE GLESv3) +endif() + add_executable(test_material_parser filament_test_material_parser.cpp ${RESGEN_SOURCE}) diff --git a/filament/test/compiler_test.cpp b/filament/test/compiler_test.cpp new file mode 100644 index 00000000000..9e914128cb1 --- /dev/null +++ b/filament/test/compiler_test.cpp @@ -0,0 +1,136 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include +#include +#include + +#include + +using namespace utils; +using namespace std::literals; + +class CompilerTest : public testing::Test { +protected: + void SetUp() override { + EGLBoolean success; + EGLint major, minor; + dpy = eglGetDisplay(EGL_DEFAULT_DISPLAY); + ASSERT_NE(dpy, EGL_NO_DISPLAY); + + EGLBoolean const initialized = eglInitialize(dpy, &major, &minor); + ASSERT_TRUE(initialized); + + EGLint const contextAttribs[] = { + EGL_CONTEXT_CLIENT_VERSION, 3, + EGL_NONE + }; + + EGLint configsCount; + EGLint configAttribs[] = { + EGL_RENDERABLE_TYPE, EGL_OPENGL_ES3_BIT_KHR, // 0 + EGL_RED_SIZE, 8, // 2 + EGL_GREEN_SIZE, 8, // 4 + EGL_BLUE_SIZE, 8, // 6 + EGL_NONE // 14 + }; + success = eglChooseConfig(dpy, configAttribs, &config, 1, &configsCount); + ASSERT_TRUE(success); + + context = eglCreateContext(dpy, config, EGL_NO_CONTEXT, contextAttribs); + ASSERT_NE(context, EGL_NO_CONTEXT); + + success = eglMakeCurrent(dpy, EGL_NO_SURFACE, EGL_NO_SURFACE, context); + ASSERT_TRUE(success); + + ASSERT_EQ(eglGetError(), EGL_SUCCESS); + } + + void TearDown() override { + eglMakeCurrent(dpy, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT); + eglDestroyContext(dpy, context); + eglTerminate(dpy); + } + +private: + EGLDisplay dpy; + EGLContext context; + EGLConfig config; +}; + +TEST_F(CompilerTest, Simple) { + auto vertex = R"( +#version 300 es +void main() +{ +})"sv; + + const char* const vs = vertex.data(); + GLint const vl = (GLint)vertex.size(); + + GLuint const vid = glCreateShader(GL_VERTEX_SHADER); + EXPECT_EQ(glGetError(), GL_NO_ERROR); + + glShaderSource(vid, 1, &vs, &vl); + EXPECT_EQ(glGetError(), GL_NO_ERROR); + + glCompileShader(vid); + EXPECT_EQ(glGetError(), GL_NO_ERROR); + + glDeleteShader(vid); + EXPECT_EQ(glGetError(), GL_NO_ERROR); +} + +TEST_F(CompilerTest, CrashPVRUniFlexCompileToHw) { + + // Some PowerVR driver crash with this shader + + auto vertex = R"( +#version 300 es + +layout(location = 0) in vec4 mesh_position; + +layout(std140) uniform FrameUniforms { + vec2 i; +} frameUniforms; + +void main() { + gl_Position = mesh_position; + gl_Position.z = dot(gl_Position.zw, frameUniforms.i); +})"sv; + + const char* const vs = vertex.data(); + GLint const vl = (GLint)vertex.size(); + + GLuint const vid = glCreateShader(GL_VERTEX_SHADER); + EXPECT_EQ(glGetError(), GL_NO_ERROR); + + glShaderSource(vid, 1, &vs, &vl); + EXPECT_EQ(glGetError(), GL_NO_ERROR); + + glCompileShader(vid); + EXPECT_EQ(glGetError(), GL_NO_ERROR); + + glDeleteShader(vid); + EXPECT_EQ(glGetError(), GL_NO_ERROR); +} + +int main(int argc, char** argv) { + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} From 1e4172b820b0ae3ae043169e42708d2f6bdc627c Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Mon, 7 Aug 2023 16:43:54 -0700 Subject: [PATCH 12/15] `RenderTarget` needs not to have a color attachment This was a somewhat arbitrary requirement, some RenderTarget could be depth only for instance. --- .../java/com/google/android/filament/RenderTarget.java | 2 -- filament/include/filament/RenderTarget.h | 2 -- filament/src/details/RenderTarget.cpp | 9 +++++---- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/android/filament-android/src/main/java/com/google/android/filament/RenderTarget.java b/android/filament-android/src/main/java/com/google/android/filament/RenderTarget.java index 2485f5c64e7..f7ebda4400e 100644 --- a/android/filament-android/src/main/java/com/google/android/filament/RenderTarget.java +++ b/android/filament-android/src/main/java/com/google/android/filament/RenderTarget.java @@ -81,8 +81,6 @@ public Builder() { /** * Sets a texture to a given attachment point. * - *

All RenderTargets must have a non-null COLOR attachment.

- * * @param attachment The attachment point of the texture. * @param texture The associated texture object. * @return A reference to this Builder for chaining calls. diff --git a/filament/include/filament/RenderTarget.h b/filament/include/filament/RenderTarget.h index 950bbb8dbac..508e1c246f2 100644 --- a/filament/include/filament/RenderTarget.h +++ b/filament/include/filament/RenderTarget.h @@ -91,8 +91,6 @@ class UTILS_PUBLIC RenderTarget : public FilamentAPI { /** * Sets a texture to a given attachment point. * - * All RenderTargets must have a non-null COLOR attachment. - * * When using a DEPTH attachment, it is important to always disable post-processing * in the View. Failing to do so will cause the DEPTH attachment to be ignored in most * cases. diff --git a/filament/src/details/RenderTarget.cpp b/filament/src/details/RenderTarget.cpp index bb8a0854182..f1084a3cf85 100644 --- a/filament/src/details/RenderTarget.cpp +++ b/filament/src/details/RenderTarget.cpp @@ -68,10 +68,11 @@ RenderTarget* RenderTarget::Builder::build(Engine& engine) { using backend::TextureUsage; const FRenderTarget::Attachment& color = mImpl->mAttachments[(size_t)AttachmentPoint::COLOR0]; const FRenderTarget::Attachment& depth = mImpl->mAttachments[(size_t)AttachmentPoint::DEPTH]; - ASSERT_PRECONDITION(color.texture, "COLOR0 attachment not set"); - ASSERT_PRECONDITION(color.texture->getUsage() & TextureUsage::COLOR_ATTACHMENT, - "Texture usage must contain COLOR_ATTACHMENT"); + if (color.texture) { + ASSERT_PRECONDITION(color.texture->getUsage() & TextureUsage::COLOR_ATTACHMENT, + "Texture usage must contain COLOR_ATTACHMENT"); + } if (depth.texture) { ASSERT_PRECONDITION(depth.texture->getUsage() & TextureUsage::DEPTH_ATTACHMENT, @@ -135,7 +136,7 @@ FRenderTarget::FRenderTarget(FEngine& engine, const RenderTarget::Builder& build for (size_t i = 0; i < MRT::MAX_SUPPORTED_RENDER_TARGET_COUNT; i++) { Attachment const& attachment = mAttachments[i]; if (attachment.texture) { - TargetBufferFlags targetBufferBit = getTargetBufferFlagsAt(i); + TargetBufferFlags const targetBufferBit = getTargetBufferFlagsAt(i); mAttachmentMask |= targetBufferBit; setAttachment(mrt[i], (AttachmentPoint)i); if (any(attachment.texture->getUsage() & From 018d6f877f18e5e64f5bd352484b7e21e0aed077 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Mon, 7 Aug 2023 16:26:19 -0700 Subject: [PATCH 13/15] Workaround for some PowerVR devices The PowerVR compiler systematically crashes on some devices when `gl_Position` is written twice in the vertex shader. fixes #5118, b/190221124 --- NEW_RELEASE_NOTES.md | 1 + shaders/src/main.vs | 22 ++++++++++++++-------- shaders/src/post_process.vs | 12 +++++++++--- 3 files changed, 24 insertions(+), 11 deletions(-) diff --git a/NEW_RELEASE_NOTES.md b/NEW_RELEASE_NOTES.md index 72fdf587ac9..30faa9cc179 100644 --- a/NEW_RELEASE_NOTES.md +++ b/NEW_RELEASE_NOTES.md @@ -8,3 +8,4 @@ appropriate header in [RELEASE_NOTES.md](./RELEASE_NOTES.md). ## Release notes for next branch cut - backend: fix #6997 : picking can fail on Adreno [⚠️ **New Material Version**] +- backend: A partial workaround for PowerVR devices (#5118, b/190221124) [⚠️ **Recompile Materials**] \ No newline at end of file diff --git a/shaders/src/main.vs b/shaders/src/main.vs index de940108cf7..aba073cbd8c 100644 --- a/shaders/src/main.vs +++ b/shaders/src/main.vs @@ -154,28 +154,29 @@ void main() { #endif // !defined(USE_OPTIMIZED_DEPTH_VERTEX_SHADER) + vec4 position; #if defined(VERTEX_DOMAIN_DEVICE) // The other vertex domains are handled in initMaterialVertex()->computeWorldPosition() - gl_Position = getPosition(); + position = getPosition(); #if !defined(USE_OPTIMIZED_DEPTH_VERTEX_SHADER) #if defined(MATERIAL_HAS_CLIP_SPACE_TRANSFORM) - gl_Position = getMaterialClipSpaceTransform(material) * gl_Position; + position = getMaterialClipSpaceTransform(material) * position; #endif #endif // !USE_OPTIMIZED_DEPTH_VERTEX_SHADER #if defined(MATERIAL_HAS_VERTEX_DOMAIN_DEVICE_JITTERED) // Apply the clip-space transform which is normally part of the projection - gl_Position.xy = gl_Position.xy * frameUniforms.clipTransform.xy + (gl_Position.w * frameUniforms.clipTransform.zw); + position.xy = position.xy * frameUniforms.clipTransform.xy + (position.w * frameUniforms.clipTransform.zw); #endif #else - gl_Position = getClipFromWorldMatrix() * getWorldPosition(material); + position = getClipFromWorldMatrix() * getWorldPosition(material); #endif #if defined(VERTEX_DOMAIN_DEVICE) // GL convention to inverted DX convention (must happen after clipSpaceTransform) - gl_Position.z = gl_Position.z * -0.5 + 0.5; + position.z = position.z * -0.5 + 0.5; #endif #if defined(VARIANT_HAS_VSM) @@ -197,15 +198,20 @@ void main() { #endif // this must happen before we compensate for vulkan below - vertex_position = gl_Position; + vertex_position = position; #if defined(TARGET_VULKAN_ENVIRONMENT) // In Vulkan, clip space is Y-down. In OpenGL and Metal, clip space is Y-up. - gl_Position.y = -gl_Position.y; + position.y = -position.y; #endif #if !defined(TARGET_VULKAN_ENVIRONMENT) && !defined(TARGET_METAL_ENVIRONMENT) // This is not needed in Vulkan or Metal because clipControl is always (1, 0) - gl_Position.z = dot(gl_Position.zw, frameUniforms.clipControl); + // (We don't use a dot() here because it workaround a spirv-opt optimization that in turn + // causes a crash on PowerVR, see #5118) + position.z = position.z * frameUniforms.clipControl.x + position.w * frameUniforms.clipControl.y; #endif + + // some PowerVR drivers crash when gl_Position is written more than once + gl_Position = position; } diff --git a/shaders/src/post_process.vs b/shaders/src/post_process.vs index 893a7da00f3..602b3ee1a59 100644 --- a/shaders/src/post_process.vs +++ b/shaders/src/post_process.vs @@ -5,14 +5,17 @@ void main() { inputs.normalizedUV = position.xy * 0.5 + 0.5; - gl_Position = getPosition(); + vec4 position = getPosition(); + // GL convention to inverted DX convention - gl_Position.z = gl_Position.z * -0.5 + 0.5; + position.z = position.z * -0.5 + 0.5; // Adjust clip-space #if !defined(TARGET_VULKAN_ENVIRONMENT) && !defined(TARGET_METAL_ENVIRONMENT) // This is not needed in Vulkan or Metal because clipControl is always (1, 0) - gl_Position.z = dot(gl_Position.zw, frameUniforms.clipControl); + // (We don't use a dot() here because it workaround a spirv-opt optimization that in turn + // causes a crash on PowerVR, see #5118) + position.z = position.z * frameUniforms.clipControl.x + position.w * frameUniforms.clipControl.y; #endif // Invoke user code @@ -31,4 +34,7 @@ void main() { #if defined(VARIABLE_CUSTOM3) VARIABLE_CUSTOM_AT3 = inputs.VARIABLE_CUSTOM3; #endif + + // some PowerVR drivers crash when gl_Position is written more than once + gl_Position = position; } From bcfdf2f70dd82590bb2b5b9ca21ddeddadda94c0 Mon Sep 17 00:00:00 2001 From: Powei Feng Date: Wed, 9 Aug 2023 10:21:08 -0700 Subject: [PATCH 14/15] Release Filament 1.40.5 --- NEW_RELEASE_NOTES.md | 2 -- README.md | 4 ++-- RELEASE_NOTES.md | 5 +++++ android/gradle.properties | 2 +- ios/CocoaPods/Filament.podspec | 4 ++-- libs/filabridge/include/filament/MaterialEnums.h | 2 +- web/filament-js/package.json | 2 +- 7 files changed, 12 insertions(+), 9 deletions(-) diff --git a/NEW_RELEASE_NOTES.md b/NEW_RELEASE_NOTES.md index 30faa9cc179..4a1a9c7fa7e 100644 --- a/NEW_RELEASE_NOTES.md +++ b/NEW_RELEASE_NOTES.md @@ -7,5 +7,3 @@ for next branch cut* header. appropriate header in [RELEASE_NOTES.md](./RELEASE_NOTES.md). ## Release notes for next branch cut -- backend: fix #6997 : picking can fail on Adreno [⚠️ **New Material Version**] -- backend: A partial workaround for PowerVR devices (#5118, b/190221124) [⚠️ **Recompile Materials**] \ No newline at end of file diff --git a/README.md b/README.md index 99b310357df..990065fc80e 100644 --- a/README.md +++ b/README.md @@ -31,7 +31,7 @@ repositories { } dependencies { - implementation 'com.google.android.filament:filament-android:1.40.4' + implementation 'com.google.android.filament:filament-android:1.40.5' } ``` @@ -51,7 +51,7 @@ Here are all the libraries available in the group `com.google.android.filament`: iOS projects can use CocoaPods to install the latest release: ``` -pod 'Filament', '~> 1.40.4' +pod 'Filament', '~> 1.40.5' ``` ### Snapshots diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index c1827db87a2..9af351b26fc 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -7,6 +7,11 @@ A new header is inserted each time a *tag* is created. Instead, if you are authoring a PR for the main branch, add your release note to [NEW_RELEASE_NOTES.md](./NEW_RELEASE_NOTES.md). +## v1.41.0 + +- backend: fix #6997 : picking can fail on Adreno [⚠️ **New Material Version**] +- backend: A partial workaround for PowerVR devices (#5118, b/190221124) [⚠️ **Recompile Materials**] + ## v1.40.5 - backend: Disable timer queries on all Mali GPUs (fixes b/233754398) diff --git a/android/gradle.properties b/android/gradle.properties index 879f309c2ca..d017be996c6 100644 --- a/android/gradle.properties +++ b/android/gradle.properties @@ -1,5 +1,5 @@ GROUP=com.google.android.filament -VERSION_NAME=1.40.4 +VERSION_NAME=1.40.5 POM_DESCRIPTION=Real-time physically based rendering engine for Android. diff --git a/ios/CocoaPods/Filament.podspec b/ios/CocoaPods/Filament.podspec index 86e39c076d6..8a31ccd6333 100644 --- a/ios/CocoaPods/Filament.podspec +++ b/ios/CocoaPods/Filament.podspec @@ -1,12 +1,12 @@ Pod::Spec.new do |spec| spec.name = "Filament" - spec.version = "1.40.4" + spec.version = "1.40.5" spec.license = { :type => "Apache 2.0", :file => "LICENSE" } spec.homepage = "https://google.github.io/filament" spec.authors = "Google LLC." spec.summary = "Filament is a real-time physically based rendering engine for Android, iOS, Windows, Linux, macOS, and WASM/WebGL." spec.platform = :ios, "11.0" - spec.source = { :http => "https://github.com/google/filament/releases/download/v1.40.4/filament-v1.40.4-ios.tgz" } + spec.source = { :http => "https://github.com/google/filament/releases/download/v1.40.5/filament-v1.40.5-ios.tgz" } # Fix linking error with Xcode 12; we do not yet support the simulator on Apple silicon. spec.pod_target_xcconfig = { diff --git a/libs/filabridge/include/filament/MaterialEnums.h b/libs/filabridge/include/filament/MaterialEnums.h index 9ade8a98a4d..3106cbfb4d7 100644 --- a/libs/filabridge/include/filament/MaterialEnums.h +++ b/libs/filabridge/include/filament/MaterialEnums.h @@ -28,7 +28,7 @@ namespace filament { // update this when a new version of filament wouldn't work with older materials -static constexpr size_t MATERIAL_VERSION = 40; +static constexpr size_t MATERIAL_VERSION = 41; /** * Supported shading models diff --git a/web/filament-js/package.json b/web/filament-js/package.json index 4745c2a343f..b08a19a8b53 100644 --- a/web/filament-js/package.json +++ b/web/filament-js/package.json @@ -1,6 +1,6 @@ { "name": "filament", - "version": "1.40.4", + "version": "1.40.5", "description": "Real-time physically based rendering engine", "main": "filament.js", "module": "filament.js", From 175c9f9966b888c84be52de56bfdff1fba41d6f1 Mon Sep 17 00:00:00 2001 From: Powei Feng Date: Wed, 9 Aug 2023 10:43:02 -0700 Subject: [PATCH 15/15] Bump version to 1.41.0 --- README.md | 4 ++-- android/gradle.properties | 2 +- ios/CocoaPods/Filament.podspec | 4 ++-- web/filament-js/package.json | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 990065fc80e..c0be01cf2fd 100644 --- a/README.md +++ b/README.md @@ -31,7 +31,7 @@ repositories { } dependencies { - implementation 'com.google.android.filament:filament-android:1.40.5' + implementation 'com.google.android.filament:filament-android:1.41.0' } ``` @@ -51,7 +51,7 @@ Here are all the libraries available in the group `com.google.android.filament`: iOS projects can use CocoaPods to install the latest release: ``` -pod 'Filament', '~> 1.40.5' +pod 'Filament', '~> 1.41.0' ``` ### Snapshots diff --git a/android/gradle.properties b/android/gradle.properties index d017be996c6..90ffbf3a907 100644 --- a/android/gradle.properties +++ b/android/gradle.properties @@ -1,5 +1,5 @@ GROUP=com.google.android.filament -VERSION_NAME=1.40.5 +VERSION_NAME=1.41.0 POM_DESCRIPTION=Real-time physically based rendering engine for Android. diff --git a/ios/CocoaPods/Filament.podspec b/ios/CocoaPods/Filament.podspec index 8a31ccd6333..0f762db725e 100644 --- a/ios/CocoaPods/Filament.podspec +++ b/ios/CocoaPods/Filament.podspec @@ -1,12 +1,12 @@ Pod::Spec.new do |spec| spec.name = "Filament" - spec.version = "1.40.5" + spec.version = "1.41.0" spec.license = { :type => "Apache 2.0", :file => "LICENSE" } spec.homepage = "https://google.github.io/filament" spec.authors = "Google LLC." spec.summary = "Filament is a real-time physically based rendering engine for Android, iOS, Windows, Linux, macOS, and WASM/WebGL." spec.platform = :ios, "11.0" - spec.source = { :http => "https://github.com/google/filament/releases/download/v1.40.5/filament-v1.40.5-ios.tgz" } + spec.source = { :http => "https://github.com/google/filament/releases/download/v1.41.0/filament-v1.41.0-ios.tgz" } # Fix linking error with Xcode 12; we do not yet support the simulator on Apple silicon. spec.pod_target_xcconfig = { diff --git a/web/filament-js/package.json b/web/filament-js/package.json index b08a19a8b53..8f6ce074e75 100644 --- a/web/filament-js/package.json +++ b/web/filament-js/package.json @@ -1,6 +1,6 @@ { "name": "filament", - "version": "1.40.5", + "version": "1.41.0", "description": "Real-time physically based rendering engine", "main": "filament.js", "module": "filament.js",