Update returnability of objects based on new encroachment rules
Review Request #56 - Created Dec. 22, 2010 and submitted
| Merov Linden | Reviewers | ||
| viewer | |||
| STORM-807 | andrew_linden | ||
| None | viewer-development | ||
The object-vs-parcel overlap test is done by building axis-aligned bounding boxes (AABB) about each prim of the selected objects and then checking for overlap between those boxes and self- and group-owned parcels.
Expand AllPosted (Dec. 22, 2010, 9:42 a.m.)
Few things I'd like to see fixed before moving to integration: llversionserver.h : this change is irrelevant to the issue, take out. llversionviewer.h : same llregionflags.h : are all those changes relevant to the issue? I can see the interest of REGION_FLAGS_ALLOW_RETURN_ENCROACHING_OBJECT but what about all the other flags suppressed or commented out? Anything in there changed that shouldn't? InfoPlist.strings : don't change that, let release do this when it's necessary to release a version Info-SecondLife.plist : same viewerRes.rc : same
Posted (Dec. 22, 2010, 10:45 a.m.)
I have the feeling that this notion of 'encroachesOwned' is going to cause a lot of trouble in the case of sculptures. At the very least you should use use an aligned bb that is the minimal bb for anything *visible* of a prim. In the case of the sculptures this requires a special function to find the min/max coordinates of all the points used. The same story holds for path cut-off (mega) prims. Ie, if I resize a megaprim (the ONLY way to resize a mega prim is by using one of those prim torture tricks) then it's very possible one ends up with a prim that is just 1/3 of it's "size". It wouldn't be fair when a house that clearly ends on ones own parcel is "detected" to encroach another parcel, just because the viewer code is too lame to take path cutting and sphere dimples etc into account.
Review request changed
Updated (Dec. 22, 2010, 11:54 p.m.)
-
- added Diff r2
Andrew's update with comments from Merov's taken into account
Posted (Dec. 22, 2010, 11:54 p.m.)
indra/llcommon/llversionviewer.h shouldn't be modified
Posted (Dec. 23, 2010, 4:12 a.m.)
-
indra/llcommon/llversionviewer.h (Diff revision 2) -
This is wrong - com.secondlife.indra.viewer is correct.
-
indra/llmessage/llregionflags.h (Diff revision 2) -
shouldn't these lines just be deleted? commented out code is confusing
-
indra/llmessage/llregionflags.h (Diff revision 2) -
delete?
-
indra/llmessage/llregionflags.h (Diff revision 2) -
delete
-
indra/llmessage/llregionflags.h (Diff revision 2) -
why add commented out code?
-
indra/llmessage/llregionflags.h (Diff revision 2) -
delete
-
indra/newview/llviewerregion.h (Diff revision 2) -
please add doxygen-style comments
Review request changed
Updated (Jan. 10, 2011, 3:36 p.m.)
-
- added Diff r3
Latest diff from most recent andrew_linden repo
Good! Only minor typos and code style issues (see review details). About lines commented out, please prefer deletion unless there's a good reason for it which then should be mentioned in comments.
-
indra/llmath/llbbox.cpp (Diff revision 3) -
Typo : change "rotiation" to "rotation"
-
indra/llmessage/llregionflags.h (Diff revision 3) -
Suppress legacy code, don't leave it commented out
-
indra/llmessage/llregionflags.h (Diff revision 3) -
Suppress old code, don't leave it commented out
-
indra/llmessage/llregionflags.h (Diff revision 3) -
Please suppress (though it was already commented out...)
-
indra/llmessage/llregionflags.h (Diff revision 3) -
Suppress etc...
-
indra/llmessage/llregionflags.h (Diff revision 3) -
Suppress etc...
-
indra/newview/llviewerobject.h (Diff revision 3) -
Typo: one "it" too many
-
indra/newview/llviewerparceloverlay.cpp (Diff revision 3) -
Code style: - use "{ }" for each for nested loop and each if statement - use "()" in "||" statement
Other reviews