Xcode16 build fix #2585
Xcode16 build fix #2585
Conversation
marchcat
force-pushed
the
marchcat/xcode-16
branch
6 times, most recently
from
d7192fe
to
8e3f5b3
Compare
|
I see you are trying to enable C++20 on macOS at least. Are you going to fix the broken hash generation in llgltfmaterial and llmaterial we discovered in #2330? |
|
That should be the next step. |
|
Ok, but you should check |
marchcat
force-pushed
the
marchcat/xcode-16
branch
3 times, most recently
from
707ffc3
to
42f7774
Compare
marchcat
force-pushed
the
marchcat/xcode-16
branch
from
42f7774
to
576c558
Compare
indra/newview/llviewerwindow.cpp
Outdated
| @@ -1331,7 +1331,7 @@ LLWindowCallbacks::DragNDropResult LLViewerWindow::handleDragNDrop( LLWindow *wi | |||
| // Check the whitelist, if there's media (otherwise just show it) | |||
| if (te->getMediaData() == NULL || te->getMediaData()->checkCandidateUrl(url)) | |||
| { | |||
| if ( obj != mDragHoveredObject) | |||
| if (obj != mDragHoveredObject.get()) | |||
There was a problem hiding this comment.
Let's pull in Ansariel's free functions to compare (T* == LLPointer<T>) and !=.
There was a problem hiding this comment.
They are already in the develop branch (d91d39f). The problem here is that obj is a LLVOVolume pointer and mDragHoveredObject is an LLPointer<LLViewerObject>, so the overloaded operator doesn't work.
There was a problem hiding this comment.
If we're comparing pointers to different types in the same class hierarchy, shouldn't we formally be downcasting one of them to match the other? We don't use a lot of multiple inheritance, but a pointer to object A's base class doesn't necessarily have the same bit pattern as a pointer to A.
There was a problem hiding this comment.
That would probably be a smart thing to do.
There was a problem hiding this comment.
I should have realized the language takes care of that.
That being the case, I'll try tweaking the (T* == LLPointer<T>) comparisons to accept a raw pointer to arbitrary type, letting the compiler produce an error if the two pointer types can't be compared.
That is, both `LLPointer::operator==()` and `operator!=()` and the free function `operator==()` and `operator!=()` now accept pointer types other than the type of the subject `LLPointer`, letting the compiler apply implicit pointer conversions or diagnose an error.
Instead of restating the whole class, requiring changes to be made in parallel (which has already failed), just make a template alias.
This affords strong exception safety. Also, eliminating the conditional may improve speed.
Given templated constructors and assignment operators, it's tempting to remove specific constructors and assignment operators with the same LLPointer<Type> parameters. That turns out to be a mistake. Add comments to warn future maintainers.
Generalize LLPointer's comparison operators to allow comparable LLPointer types.
|
Excuse me while I'm capturing this PR for an unrelated, but kinda similar question: Is the Linux runner using Ubuntu 22.04? And if so, can we get a Ubuntu 24.04 / GCC13 compatibility pass? (Buggy) GCC13 gets really angry about some templating stuff that was added with the Lua feature merge. |
we will certainly want to keep our distributed builds on 22.04 for some time. we can do work for compatibility with newer compilers, but moving to a newer glibc based system will limit our distribution compatibility even further |
Now the lerp() in global namespace is std::lerp(), but it remains true that for some calls to std::lerp(), MSVC issues fatal argument conversion warnings. In those places, call flerp() (our historic lerp()) instead.
|
This PR actually does turn on C++20. Once it succeeds, do we need further discussion with anyone before merging to develop? |
What about the hash calculation for BP and GLTF materials? Is that fixed or does it need more attention? See #2330 (comment) |
|
Dave was running some head-to-head performance tests with various hash tactics; I don't know where he committed the results of his work. |
|
But does the test succeed now? Previously xcode seem to have changed something to the memory alignment of the said material classes, causing the test to fail. |
Looking at the cases that got changed to flerp, could it be VS got mad because in all those cases you are mixing parameter types (int/float)?
|
|
FYI: C++23 builds fine with VS 🫡 |
|
build with all the changes: https://github.com/secondlife/viewer/releases/tag/Second_Life_Release%23d3833b6 |
This allows removing #include "llerror.h" from llpointer.h. Also remove #include "llmutex.h" as a heavy way to get <boost/functional/hash.hpp>. That requires adding #include "llmutex.h" to llimage.h, llnotifications.h, llwatchdog.cpp and llvolumemgr.cpp, which were inheriting it from llpointer.h.
Instead of using strdup() and free() to store comment_text in JPEG2KEncode, store a std::string. Similarly, instead of manually managing pointers to encoder, decoder, image, stream and codestream_info in JPEG2KDecode and JPEG2KEncode, use std::unique_ptr for each, specifying their respective deleter functions.
owning_ptr<T> adapts std::unique_ptr<T> to be a better drop-in replacement for a legacy class that formerly stored plain T* data members, and explicitly destroyed them using domain-specific destructor functions. Directly substituting std::unique_ptr into JPEG2KEncode and JPEG2KDecode was cumbersome because every such pointer declaration required a redundant template parameter describing the deleter function passed into its constructor. Moreover, it required lots of little syntax tweaks: changing every assignment to a reset() call, changing every reference to a get() call. Using owning_ptr<T> allows us to leave the code more or less as it was before, save that assignment and destruction automatically handle the previous referenced T instance.
Clean up llpointer.h per previous discussions.
|
A new branch tip build for evaluation |
No description provided.