[Model] [macOS] Add support for rendering model resources
Created attachment 432598 [details] Patch
<rdar://problem/79968206>
Created attachment 432599 [details] Patch
Comment on attachment 432599 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=432599&action=review > Source/WebCore/Modules/model-element/HTMLModelElement.mm:29 > +#if ENABLE(MODEL_ELEMENT) We shouldn't use .mm for cross platform code like this. Please keep this a .cpp file and factor out the objective-c bits into their own classes.
(In reply to Sam Weinig from comment #4) > Comment on attachment 432599 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=432599&action=review > > > Source/WebCore/Modules/model-element/HTMLModelElement.mm:29 > > +#if ENABLE(MODEL_ELEMENT) > > We shouldn't use .mm for cross platform code like this. Please keep this a > .cpp file and factor out the objective-c bits into their own classes. Right, so add a Source/WebCore/Modules/model-element/HTMLModelElementCocoa.mm with these bits?
Created attachment 432623 [details] Patch
Comment on attachment 432623 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=432623&action=review No test? > Source/WebCore/ChangeLog:12 > + <model> element is created and message the UIProcess through the ChromeClient provinding the UUID generated for it. When Typo provinding > Source/WebCore/Modules/model-element/HTMLModelElementCocoa.mm:43 > +#include <wtf/UUID.h> > +SOFT_LINK_PRIVATE_FRAMEWORK(AssetViewer); Add a space between these lines. > Source/WebCore/Modules/model-element/HTMLModelElementCocoa.mm:66 > + auto fileName = FileSystem::encodeForFileName(createCanonicalUUIDString()) + ".usdz"; What about if it is a .reality file? > Source/WebCore/Modules/model-element/HTMLModelElementCocoa.mm:74 > + auto file = FileSystem::openFile(filePath, FileSystem::FileOpenMode::Write); > + if (file <= 0) > + return; > + > + FileSystem::writeToFile(file, m_data->data(), m_data->size()); > + FileSystem::closeFile(file); > + m_filePath = filePath; Can you put a note in here that this is a temporary solution and we don't want to write to files? > Source/WebCore/Modules/model-element/HTMLModelElementCocoa.mm:92 > + FloatSize size; > + if (auto* renderer = this->renderer()) > + size = renderer->absoluteBoundingBoxRect(false).size(); If there isn't a renderer you probably just want to exit. if (!this->renderer()) return; auto size = this->renderer()->aBB... > Source/WebKit/UIProcess/Cocoa/ModelElementControllerCocoa.mm:134 > + else > + iterator->value = preview; I didn't know you could do this! Very nice.
(In reply to Dean Jackson from comment #7) > Comment on attachment 432623 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=432623&action=review > > > Source/WebCore/Modules/model-element/HTMLModelElementCocoa.mm:66 > > + auto fileName = FileSystem::encodeForFileName(createCanonicalUUIDString()) + ".usdz"; > > What about if it is a .reality file? Filed bug 227568, will add a reference to it in the commit. > > Source/WebCore/Modules/model-element/HTMLModelElementCocoa.mm:74 > > + auto file = FileSystem::openFile(filePath, FileSystem::FileOpenMode::Write); > > + if (file <= 0) > > + return; > > + > > + FileSystem::writeToFile(file, m_data->data(), m_data->size()); > > + FileSystem::closeFile(file); > > + m_filePath = filePath; > > Can you put a note in here that this is a temporary solution and we don't > want to write to files? Filed bug 227567 and adding a reference to it in the commit.
Committed r279451 (239307@main): <https://commits.webkit.org/239307@main>
(In reply to Antoine Quint from comment #5) > (In reply to Sam Weinig from comment #4) > > Comment on attachment 432599 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=432599&action=review > > > > > Source/WebCore/Modules/model-element/HTMLModelElement.mm:29 > > > +#if ENABLE(MODEL_ELEMENT) > > > > We shouldn't use .mm for cross platform code like this. Please keep this a > > .cpp file and factor out the objective-c bits into their own classes. > > Right, so add a > Source/WebCore/Modules/model-element/HTMLModelElementCocoa.mm with these > bits? I usually try to add some c++ wrapper class in its own files instead to avoid having multiple implementation files for the same header which is not something that is very understandable.