secondlife / jira-archive

2 stars 0 forks source link

[BUG-234746] [PBR] Reflection probe type and prim type desyncronize #11615

Open sl-service-account opened 9 months ago

sl-service-account commented 9 months ago

What just happened?

1) Enabled Render Reflection Probe Volumes in Build options 2) Created a new prim (cube) 3) Enabled "Reflection Probe" in the Features tab of the Build dialog. The reflection probe type defaulted to "Sphere" and the object prim type was changed from Box to Sphere. 4) Changed the Reflection Probe type to "Box". The object prim type was changed to Box. 5) Disabled "Reflection Probe" in the Features tab of the Build dialog. The prim type was left as a box. 6) Re-enabled "Reflection Probe" in the Features tab of the Build dialog. The reflection probe type is set to "Box" and the reflection probe volume renders as a yellow box. However, the prim type is set to "Sphere" and the selected object outline is rendered as a sphere.

What were you doing when it happened?

Toggling the reflection probe setting on and off.

What were you expecting to happen instead?

The prim object type and reflection probe type should be the same when reflection probes are re-enabled for a prim.

Other information

See attached screenshot

Attachments

Original Jira Fields | Field | Value | | ------------- | ------------- | | Issue | BUG-234746 | | Summary | [PBR] Reflection probe type and prim type desyncronize | | Type | Bug | | Priority | Unset | | Status | Accepted | | Resolution | Triaged | | Labels | pbr | | Reporter | Thornotter (thornotter) | | Created at | 2023-11-30T02:34:08Z | | Updated at | 2023-12-07T17:19:12Z | ``` { 'Build Id': 'unset', 'Business Unit': ['Platform'], 'Date of First Response': '2023-11-29T21:18:19.050-0600', "Is there anything you'd like to add?": 'See attached screenshot', 'ReOpened Count': 0.0, 'Severity': 'Unset', 'System': 'SL Viewer', 'Target Viewer Version': 'viewer-development', 'What just happened?': '1) Enabled Render Reflection Probe Volumes in Build options\r\n2) Created a new prim (cube)\r\n3) Enabled "Reflection Probe" in the Features tab of the Build dialog. The reflection probe type defaulted to "Sphere" and the object prim type was changed from Box to Sphere.\r\n4) Changed the Reflection Probe type to "Box". The object prim type was changed to Box.\r\n5) Disabled "Reflection Probe" in the Features tab of the Build dialog. The prim type was left as a box.\r\n6) Re-enabled "Reflection Probe" in the Features tab of the Build dialog. The reflection probe type is set to "Box" and the reflection probe volume renders as a yellow box. However, the prim type is set to "Sphere" and the selected object outline is rendered as a sphere.', 'What were you doing when it happened?': 'Toggling the reflection probe setting on and off.', 'What were you expecting to happen instead?': 'The prim object type and reflection probe type should be the same when reflection probes are re-enabled for a prim.', 'Where': 'http://maps.secondlife.com/secondlife/Enchantment/', } ```
sl-service-account commented 9 months ago

Zi Ree commented at 2023-11-30T03:18:19Z

This is what Firestorm uses to fix this:


diff --git a/indra/newview/llpanelvolume.cpp b/indra/newview/llpanelvolume.cpp
index f6c369c..7d66748 100644
--- a/indra/newview/llpanelvolume.cpp
+++ b/indra/newview/llpanelvolume.cpp
@@ -803,6 +803,30 @@ void LLPanelVolume::doSendIsReflectionProbe(const LLSD & notification, const LLS
         { // has become a reflection probe, slam to a 10m sphere and pop up a message
             // warning people about the pitfalls of reflection probes

+            // <FS:Zi> Let's not just slam a sphere on it if the original prim was a box-type
+            //         prim, but instead keep its shape and update the interface accordingly.
+            U8 probe_path_type = LL_PCODE_PATH_CIRCLE;
+            U8 probe_profile_type = LL_PCODE_PROFILE_CIRCLE_HALF;
+
+            std::string volume_type;
+
+            if (objectp->getVolume()->getPathType() == LL_PCODE_PATH_LINE)
+            {
+                volobjp->setReflectionProbeIsBox(true);
+
+                probe_path_type = LL_PCODE_PATH_LINE;
+                probe_profile_type = LL_PCODE_PROFILE_SQUARE;
+
+                volume_type = "Box";
+            }
+            else
+            {
+                volume_type = "Sphere";
+            }
+
+            getChild<LLComboBox>("Probe Volume Type", true)->setValue(volume_type);
+            // </FS:Zi>
+
             auto* select_mgr = LLSelectMgr::getInstance();

             select_mgr->selectionUpdatePhantom(true);
@@ -810,8 +834,12 @@ void LLPanelVolume::doSendIsReflectionProbe(const LLSD & notification, const LLS
             select_mgr->selectionSetAlphaOnly(0.f);

             LLVolumeParams params;
-            params.getPathParams().setCurveType(LL_PCODE_PATH_CIRCLE);
-            params.getProfileParams().setCurveType(LL_PCODE_PROFILE_CIRCLE_HALF);
+            // <FS:Zi> Don't change prim boxes to spheres when turning it into a reflection probe
+            // params.getPathParams().setCurveType(LL_PCODE_PATH_CIRCLE);
+            // params.getProfileParams().setCurveType(LL_PCODE_PROFILE_CIRCLE_HALF);
+            params.getPathParams().setCurveType(probe_path_type);
+            params.getProfileParams().setCurveType(probe_profile_type);
+            // </FS:Zi>
             mObject->updateVolume(params);
         }
     }
sl-service-account commented 9 months ago

JIRAUSER341268 commented at 2023-11-30T17:58:05Z, updated at 2023-12-07T17:16:06Z

Thank you for this report, Thornotter!

And thanks Zi, are you able to provide a PR for that on Github?

sl-service-account commented 9 months ago

JIRAUSER341268 commented at 2023-12-07T17:19:12Z

Above comment updated. Zi, please provide a PR for that on GH if you'd like LL to use the same code