Adapt selection picker to URP

[thomas-tu]

Purpose of this PR

This PR updates the picking system so it properly runs on URP. We used to use an old hack in URP project: the picking system would force the BiRP to run temporarly to render the lookup texture. This is no longer something we can do due to some others packages that could react to the change of RP asset changes (example: VFX graph).

To solve this and since URP now allows render pass injection, I've created our own URP render pass and inject it when we run the picking system. URP uses HLSL so I had to migrate our CG shaders to HLSL.

Manually tested.

Links

Jira: UUM-133859

[u-pr]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪ The PR adds a new URP rendering path plus shaders and assembly definition changes, which require careful validation across multiple Unity render pipeline configurations.

🏅 Score: 70 The overall direction looks correct, but there are several high-risk integration/compatibility issues (assembly dependencies, shader name mismatch, and lifecycle/cleanup concerns) that need addressing before merge.

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Compatibility Adding unconditional assembly references to URP/Core/HDRP runtime assemblies can break compilation in projects that don’t have those packages installed; consider isolating URP-specific code into a separate asmdef (or otherwise avoiding hard references) so built-in pipeline users aren’t forced to install URP. { "name": "Unity.ProBuilder", "rootNamespace": "", "references": [ "Unity.ProBuilder.Poly2Tri", "Unity.ProBuilder.KdTree", "Unity.RenderPipelines.HighDefinition.Runtime", "Unity.RenderPipelines.Universal.Runtime", "Unity.RenderPipelines.Core.Runtime" ], Possible Issue The URP face picker shader string appears inconsistent with the actual shader name added in this PR (the shader file declares a Hidden/ProBuilder path); verify that the k_FacePickerShader (and related material paths) resolve correctly at runtime to avoid null shaders/materials. #if PB_URP_MODE static string k_EdgePickerShader = "Hidden/ProBuilder/EdgePickerURP"; static string k_VertexPickerShader = "Hidden/ProBuilder/VertexPickerURP"; static string k_FacePickerShader = "Shader Graphs/FacePickerURP"; static string k_EdgePickerMaterial = "Materials/EdgePickerURP"; static string k_FacePickerMaterial = "Materials/FacePickerURP"; static string k_VertexPickerMaterial = "Materials/VertexPickerURP"; #else static string k_EdgePickerShader = "Hidden/ProBuilder/EdgePicker"; static string k_FacePickerShader = "Hidden/ProBuilder/FacePicker"; static string k_VertexPickerShader = "Hidden/ProBuilder/VertexPicker"; static string k_EdgePickerMaterial = "Materials/EdgePicker"; static string k_FacePickerMaterial = "Materials/FacePicker"; static string k_VertexPickerMaterial = "Materials/VertexPicker"; #endif Cleanup/Leaks The URP render path subscribes to RenderPipelineManager.beginCameraRendering and allocates temporary objects, but cleanup is not protected by try/finally; failures or early exits could leave the callback subscribed and/or leak the temporary camera/RT, and using UnityEditor in a Runtime file may also break player builds unless editor-guarded. using UnityEditor; using UnityEngine.Rendering; #if PB_URP_MODE using UnityEngine.Rendering.Universal; using UObject = UnityEngine.Object; using static UnityEngine.Rendering.RenderPipeline; #endif namespace UnityEngine.ProBuilder { internal partial class SelectionPickerRenderer { internal class SelectionPickerRendererURP : ISelectionPickerRenderer { public static Camera temporaryCamera; /// <summary> /// Render the camera with a replacement shader and return the resulting image. /// RenderTexture is always initialized with no gamma conversion (RenderTextureReadWrite.Linear) /// </summary> /// <param name="camera"></param> /// <param name="shader"></param> /// <param name="tag"></param> /// <param name="width"></param> /// <param name="height"></param> /// <returns></returns> public Texture2D RenderLookupTexture( Camera camera, Shader shader, string tag, int width = -1, int height = -1) { #if PB_URP_MODE bool autoSize = width < 0 || height < 0; int _width = autoSize ? (int)camera.pixelRect.width : width; int _height = autoSize ? (int)camera.pixelRect.height : height; GameObject go = new GameObject(); Camera renderCam = go.AddComponent<Camera>(); temporaryCamera = renderCam; renderCam.CopyFrom(camera); renderCam.renderingPath = RenderingPath.Forward; renderCam.enabled = false; renderCam.clearFlags = CameraClearFlags.SolidColor; renderCam.backgroundColor = Color.white; renderCam.allowHDR = false; renderCam.allowMSAA = false; renderCam.forceIntoRenderTexture = true; renderCam.GetUniversalAdditionalCameraData(); RenderTextureDescriptor descriptor = new RenderTextureDescriptor() { width = _width, height = _height, colorFormat = renderTextureFormat, autoGenerateMips = false, depthBufferBits = 16, dimension = UnityEngine.Rendering.TextureDimension.Tex2D, enableRandomWrite = false, memoryless = RenderTextureMemoryless.None, sRGB = false, useMipMap = false, volumeDepth = 1, msaaSamples = 1 }; RenderTexture rt = RenderTexture.GetTemporary(descriptor); #if PB_DEBUG /* Debug.Log(string.Format("antiAliasing {0}\nautoGenerateMips {1}\ncolorBuffer {2}\ndepth {3}\ndepthBuffer {4}\ndimension {5}\nenableRandomWrite {6}\nformat {7}\nheight {8}\nmemorylessMode {9}\nsRGB {10}\nuseMipMap {11}\nvolumeDepth {12}\nwidth {13}", RenderTexture.active.antiAliasing, RenderTexture.active.autoGenerateMips, RenderTexture.active.colorBuffer, RenderTexture.active.depth, RenderTexture.active.depthBuffer, RenderTexture.active.dimension, RenderTexture.active.enableRandomWrite, RenderTexture.active.format, RenderTexture.active.height, RenderTexture.active.memorylessMode, RenderTexture.active.sRGB, RenderTexture.active.useMipMap, RenderTexture.active.volumeDepth, RenderTexture.active.width)); */ #endif var request = new StandardRequest() { destination = rt }; RenderPipelineManager.beginCameraRendering += CustomRenderPass; if (RenderPipeline.SupportsRenderRequest(renderCam, request) == false) Debug.LogWarning("RenderRequest not supported."); RenderPipeline.SubmitRenderRequest<StandardRequest>(renderCam, request); RenderTexture prev = RenderTexture.active; RenderTexture.active = rt; Texture2D img = new Texture2D(_width, _height, textureFormat, false, false); img.ReadPixels(new Rect(0, 0, _width, _height), 0, 0); img.Apply(); RenderTexture.active = prev; RenderTexture.ReleaseTemporary(rt); RenderPipelineManager.beginCameraRendering -= CustomRenderPass; temporaryCamera = null; UObject.DestroyImmediate(go); return img;

Update review

---

🤖 Helpful? Please react with 👍/👎 | Questions❓Please reach out in Slack #ask-u-pr

[u-pr]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Fix shader name mismatch Make the URP face picker shader identifier consistent with the actual shader name to prevent Shader.Find/material creation from failing. The added URP shader file declares Hidden/ProBuilder/FacePickerURP, so use that here as well. Runtime/Core/BuiltinMaterials.cs [73-75] static string k_EdgePickerShader = "Hidden/ProBuilder/EdgePickerURP"; static string k_VertexPickerShader = "Hidden/ProBuilder/VertexPickerURP"; -static string k_FacePickerShader = "Shader Graphs/FacePickerURP"; +static string k_FacePickerShader = "Hidden/ProBuilder/FacePickerURP"; Suggestion importance[1-10]: 9 __ Why: The k_FacePickerShader variable is assigned a path ("Shader Graphs/FacePickerURP") that does not match the actual shader name defined in FacePickerURP.shader ("Hidden/ProBuilder/FacePickerURP"). This discrepancy will cause the shader lookup to fail at runtime, breaking the face picking functionality when using URP.	High
	Always clean up temporary rendering resources Ensure render pipeline event unsubscription and temporary resource cleanup happen even if rendering/reading pixels throws. Wrap the render request + readback in a try/finally that always restores RenderTexture.active, releases rt, unsubscribes, and destroys the temporary camera object. Runtime/Core/SelectionPickerRendererURP.cs [97-116] +RenderTexture prev = RenderTexture.active; RenderPipelineManager.beginCameraRendering += CustomRenderPass; -if (RenderPipeline.SupportsRenderRequest(renderCam, request) == false) - Debug.LogWarning("RenderRequest not supported."); +try +{ + if (RenderPipeline.SupportsRenderRequest(renderCam, request) == false) + Debug.LogWarning("RenderRequest not supported."); -RenderPipeline.SubmitRenderRequest<StandardRequest>(renderCam, request); -RenderTexture prev = RenderTexture.active; -RenderTexture.active = rt; + RenderPipeline.SubmitRenderRequest<StandardRequest>(renderCam, request); -Texture2D img = new Texture2D(_width, _height, textureFormat, false, false); -img.ReadPixels(new Rect(0, 0, _width, _height), 0, 0); -img.Apply(); + RenderTexture.active = rt; -RenderTexture.active = prev; -RenderTexture.ReleaseTemporary(rt); -RenderPipelineManager.beginCameraRendering -= CustomRenderPass; -temporaryCamera = null; -UObject.DestroyImmediate(go); + Texture2D img = new Texture2D(_width, _height, textureFormat, false, false); + img.ReadPixels(new Rect(0, 0, _width, _height), 0, 0); + img.Apply(); -return img; + return img; +} +finally +{ + RenderTexture.active = prev; + RenderTexture.ReleaseTemporary(rt); + RenderPipelineManager.beginCameraRendering -= CustomRenderPass; + temporaryCamera = null; + UObject.DestroyImmediate(go); +} Suggestion importance[1-10]: 7 __ Why: Implementing a try/finally block ensures that resources such as the temporary RenderTexture are released, event subscriptions are removed, and the temporary GameObject is destroyed even if an exception occurs during the rendering or pixel reading process. This enhances the robustness of the selection picking logic.	Medium
More suggestions

---

🤖 Helpful? Please react with 👍/👎 | Questions❓Please reach out in Slack #ask-u-pr

[codecov-github-com]

Codecov Report

Attention: Patch coverage is 66.66667% with 3 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
Runtime/Core/SelectionPickerRendererURP.cs	0.00%	3 Missing ⚠️

@@           Coverage Diff           @@
##           master     #657   +/-   ##
=======================================
  Coverage   36.05%   36.06%           
=======================================
  Files         277      278    +1     
  Lines       34909    34916    +7     
=======================================
+ Hits        12588    12592    +4     
- Misses      22321    22324    +3     

Flag	Coverage Δ
probuilder_MacOS_6000.0	34.58% <66.66%> (+<0.01%)	⬆️
probuilder_MacOS_6000.3	34.58% <66.66%> (+<0.01%)	⬆️
probuilder_MacOS_6000.4	34.57% <66.66%> (+<0.01%)	⬆️
probuilder_MacOS_6000.5	34.57% <66.66%> (+<0.01%)	⬆️
probuilder_MacOS_6000.6	34.57% <66.66%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
Runtime/Core/BuiltinMaterials.cs	52.42% <100.00%> (ø)
Runtime/Core/SelectionPickerRenderer.cs	92.72% <100.00%> (+0.05%)	⬆️
Runtime/Core/SelectionPickerRendererURP.cs	0.00% <0.00%> (ø)

[unity-kristinn]

/test_plan

[u-pr]

Test Plan

• Test plan approved by PR author
• Test execution in progress
• Test execution complete
• Ready for merge

---

Summary of Changes & Risk Assessment

Summary of Changes

This PR transitions the ProBuilder selection picking system to a native URP implementation. It replaces a legacy "pipeline-switching" hack with a dedicated ScriptableRenderPass using the RenderGraph API and migrates picking shaders from CG to HLSL (ProBuilderCG_URP.hlsl). It also introduces SelectionPickerRendererURP to handle the rendering of lookup textures within the Universal Render Pipeline context.

Risk Assessment

• High Risk Areas:
  • Shader/Material Resolution: Potential null references if k_FacePickerShader name mismatch identified in PR comments isn't resolved in BuiltinMaterials.cs:L75.
  • Resource Management: Risk of GameObject or RenderTexture leaks in SelectionPickerRendererURP.cs if an exception occurs during ReadPixels, as cleanup logic is not currently in a try/finally block.
• Medium Risk Areas:
  • Cross-Pipeline Compatibility: Ensuring the picking system still functions correctly in Built-in (BiRP) and HDRP after the logic branch changes in SelectionPickerRenderer.cs:L169.
  • Asmdef Dependencies: New hard references to URP/HDRP runtime assemblies in Unity.ProBuilder.asmdef could cause compilation errors in projects missing those packages if not properly guarded.
• Low Risk Areas: Changelog accuracy and basic UI selection workflows.

---

Test Scenarios

Functional Testing

• URP Selection Verification: In a URP project, verify that Vertex, Edge, and Face selection modes work correctly on a ProBuilder mesh.
• Marquee/Rect Selection: Perform a rectangle selection in the Scene View and verify that it no longer triggers a script recompilation (the core fix for UUM-133859).
• Shader Loading: Verify that BuiltinMaterials.Init() correctly finds the new HLSL shaders (especially the Face Picker) without "Shader not found" errors in the console.

Regression Testing

• Built-in Pipeline (BiRP) Parity: Open a project using BiRP and verify that selection picking remains functional and hasn't regressed due to the logic changes in SelectionPickerRenderer.cs.
• HDRP Verification: Ensure picking still works in HDRP projects, as the initialization logic for SelectionPickerRendererHDRP was modified in SelectionPickerRenderer.cs:L168.
• Dependency Guarding: Verify that the package compiles in a minimal Unity project that does not have the URP or HDRP packages installed.

🔍 Regression Deep Dive (additional risks identified)

• Camera Modes: Test selection using an Orthographic camera in URP to validate the offset logic in ProBuilderCG_URP.hlsl:L36.
• Leak Detection: Monitor the Hierarchy during repeated selection actions in URP to ensure the temporary "New Game Object" created in SelectionPickerRendererURP is destroyed properly every time.
• Z-Fighting/Offset: Verify that edge selection doesn't suffer from excessive flickering or depth-testing issues in URP compared to BiRP.

Edge Cases

• Resolution Extremes: Test selection picking in a very small Scene View window and a 4K resolution window to ensure ReadPixels coordinates in SelectionPickerRendererURP.cs are accurate.
• Mixed Shaders: Test selection on a ProBuilder mesh that uses a custom Shader Graph material to ensure the picker pass still identifies the geometry.

---

_{💡 This test plan updates automatically when /test_plan is run on new commits. If you have any feedback, please reach out in #ai-qa}

---

🤖 Helpful? Please react with 👍/👎 | Questions❓Please reach out in Slack #ask-u-pr