New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Build webgpu module full from relative path #5802
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
I was running into the same issue for my issue-example here: #5729 I tested this with that example, but changed it to use a relative path: Error: Edit: It works for me like this: // build a full URL from a relative path
const buildUrl = (relativePath) => {
const url = new URL(window.location.href);
const basedir = url.pathname.split('/').slice(0, -1).join('/') + '/';
url.pathname = basedir + relativePath;
url.search = '';
console.log("GOT URL", url.toString());
return url.toString();
}; Output: (error is only because I Linux/Chrome still doesn't support WebGPU) |
Hi @kungfooman, Please would you submit your additional change as followup PR? Then we can all discuss. Thanks! |
@@ -174,9 +174,17 @@ class WebgpuGraphicsDevice extends GraphicsDevice { | |||
// temporary message to confirm Webgpu is being used | |||
Debug.log("WebgpuGraphicsDevice initialization .."); | |||
|
|||
// build a full URL from a relative path | |||
const buildUrl = (relativePath) => { | |||
const url = new URL(window.location.href); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure we should have a window
reference which would break in node. Can we just update the createGraphicsOption
to include the full path, and assume any path passed in is correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's fine as long it's not e.g. in a variable declaration in global scope, or do you run into this code and trigger a reference error in node?
(before it was a simple <script src=...=>
aswell and if you wanted to use that you would have already required the jsdom shim lib in node to have access to window
etc. via node)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I just wonder if we just require the path to be correct when it's passed in, rather than making the engine figure it out. The createGraphics options just has the full url. Not sure, might be missing something obvious
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
correct when it's passed in
I agree with this. At the end, the application itself passes this path in, they should be able to build a correct path easier than the engine.
Fixes #playcanvas/editor#1070
Build the full url module path from relative path because when engine is loaded with CORS-cross-origin the base url is about:blank.