Skip to content
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

Fix buildUrl relative path support #6354

Merged
merged 1 commit into from May 13, 2024

Conversation

erikdubbelboer
Copy link
Contributor

buildUrl never worked with relative paths as it would always replace the path in the url, instead of adding to it.
Using the other mode of the URL constructor we can use it to always give us a correct url:

  `new URL('glslang/glslang.js', 'https://example.com/foo/bar.html')` is `https://example.com/foo/glslang/glslang.js`
  `new URL('/glslang/glslang.js', 'https://example.com/foo/bar.html')` is `https://example.com/glslang/glslang.js`
  `new URL('glslang/glslang.js', 'https://example.com/')` is `https://example.com/glslang/glslang.js`
  `new URL('/glslang/glslang.js', 'https://example.com/')` is `https://example.com/glslang/glslang.js`

This bug currently prevents Playcanvas Editor exports from being run on a non root path with WebGPU. The editor export will pass just glslang.js as path.
So on a url like https://94176748-9ef8-42c9-a44e-a95b70ec5680.poki-gdn.com/7a8f1fc6-7162-473a-b1dc-4479ed5d60cf/index.html
the old code would turn it into https://94176748-9ef8-42c9-a44e-a95b70ec5680.poki-gdn.com/glslang.js
while the actual paths should be https://94176748-9ef8-42c9-a44e-a95b70ec5680.poki-gdn.com/7a8f1fc6-7162-473a-b1dc-4479ed5d60cf/glslang.js

I confirm I have read the contributing guidelines and signed the Contributor License Agreement.

buildUrl never worked with relative paths as it would always replace the
path in the url instead of adding to it.
Using the other mode of the URL constructor we can use it to always give
us a correct url:

  `new URL('glslang/glslang.js', 'https://example.com/foo/bar.html')` is `https://example.com/foo/glslang/glslang.js`
  `new URL('/glslang/glslang.js', 'https://example.com/foo/bar.html')` is `https://example.com/glslang/glslang.js`
  `new URL('glslang/glslang.js', 'https://example.com/')` is `https://example.com/glslang/glslang.js`
  `new URL('/glslang/glslang.js', 'https://example.com/')` is `https://example.com/glslang/glslang.js`

This bug currently prevents Playcanvas Editor exports from being run on
a non root path with WebGPU. It will just pass `glslang.js` as path.
So on a url like https://94176748-9ef8-42c9-a44e-a95b70ec5680.poki-gdn.com/7a8f1fc6-7162-473a-b1dc-4479ed5d60cf/
the old code would turn it into https://94176748-9ef8-42c9-a44e-a95b70ec5680.poki-gdn.com/glslang.js
whil the actual paths should be https://94176748-9ef8-42c9-a44e-a95b70ec5680.poki-gdn.com/7a8f1fc6-7162-473a-b1dc-4479ed5d60cf/glslang.js
Copy link
Member

@slimbuck slimbuck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep this is great, thanks!

@mvaligursky mvaligursky merged commit 4e14dee into playcanvas:main May 13, 2024
7 checks passed
mvaligursky pushed a commit that referenced this pull request May 14, 2024
buildUrl never worked with relative paths as it would always replace the
path in the url instead of adding to it.
Using the other mode of the URL constructor we can use it to always give
us a correct url:

  `new URL('glslang/glslang.js', 'https://example.com/foo/bar.html')` is `https://example.com/foo/glslang/glslang.js`
  `new URL('/glslang/glslang.js', 'https://example.com/foo/bar.html')` is `https://example.com/glslang/glslang.js`
  `new URL('glslang/glslang.js', 'https://example.com/')` is `https://example.com/glslang/glslang.js`
  `new URL('/glslang/glslang.js', 'https://example.com/')` is `https://example.com/glslang/glslang.js`

This bug currently prevents Playcanvas Editor exports from being run on
a non root path with WebGPU. It will just pass `glslang.js` as path.
So on a url like https://94176748-9ef8-42c9-a44e-a95b70ec5680.poki-gdn.com/7a8f1fc6-7162-473a-b1dc-4479ed5d60cf/
the old code would turn it into https://94176748-9ef8-42c9-a44e-a95b70ec5680.poki-gdn.com/glslang.js
whil the actual paths should be https://94176748-9ef8-42c9-a44e-a95b70ec5680.poki-gdn.com/7a8f1fc6-7162-473a-b1dc-4479ed5d60cf/glslang.js
slimbuck pushed a commit to slimbuck/engine that referenced this pull request May 20, 2024
buildUrl never worked with relative paths as it would always replace the
path in the url instead of adding to it.
Using the other mode of the URL constructor we can use it to always give
us a correct url:

  `new URL('glslang/glslang.js', 'https://example.com/foo/bar.html')` is `https://example.com/foo/glslang/glslang.js`
  `new URL('/glslang/glslang.js', 'https://example.com/foo/bar.html')` is `https://example.com/glslang/glslang.js`
  `new URL('glslang/glslang.js', 'https://example.com/')` is `https://example.com/glslang/glslang.js`
  `new URL('/glslang/glslang.js', 'https://example.com/')` is `https://example.com/glslang/glslang.js`

This bug currently prevents Playcanvas Editor exports from being run on
a non root path with WebGPU. It will just pass `glslang.js` as path.
So on a url like https://94176748-9ef8-42c9-a44e-a95b70ec5680.poki-gdn.com/7a8f1fc6-7162-473a-b1dc-4479ed5d60cf/
the old code would turn it into https://94176748-9ef8-42c9-a44e-a95b70ec5680.poki-gdn.com/glslang.js
whil the actual paths should be https://94176748-9ef8-42c9-a44e-a95b70ec5680.poki-gdn.com/7a8f1fc6-7162-473a-b1dc-4479ed5d60cf/glslang.js
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: graphics Graphics related issue bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants