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

Request headers in config are not sent for tiled images #761

Closed
mikepianka opened this issue Sep 22, 2022 · 9 comments
Closed

Request headers in config are not sent for tiled images #761

mikepianka opened this issue Sep 22, 2022 · 9 comments
Milestone

Comments

@mikepianka
Copy link

I noticed that when setting an authorization header via the requestHeaders config that the header does not get sent for tiled images.

https://photo-sphere-viewer.js.org/guide/config.html#requestheaders

I am dynamically setting the request header with a token using a function like this:

function changeRequestToken(token: string) {
  viewer.config.requestHeaders = (url) => {
    console.debug(`requesting asset ${url} with token ${token}`);
    return { Authorization: `Bearer ${token}` };
  };
}

If I use the standard equirectangular adapter, the header is sent as expected:
image

If I switch to the equirectangular tiled adapter, the header gets sent for the low resolution preview image:
image

But the header fails to be sent for the image tiles:
image

@mistic100
Copy link
Owner

mistic100 commented Sep 22, 2022

The standard equirectangular adapter uses a FileLoader to have direct access to the file content for XMP parsing, thus it uses the Fetch API, which supports headers.

Equirectangular tiles in the other hand uses ImageLoader to keep performances as high as possible*. This uses a standard Image which does not support headers.

What ever you are trying to do you will have to find a solution which already works with basic <img> elements. I guess the only viable solutions are cookies and query params.

* performances are not impacted by the way the file is downloaded but by the subsequent image instanciation that needs to be done if the image is downloaded as a blob, specifically this part

@mikepianka
Copy link
Author

mikepianka commented Sep 22, 2022

OK, thank you for the info. That makes more sense now with that context. I had started looking through the code figuring there had to be a way that regular equirectangular images were handled that was the same as the low res preview.

I found some issues about this in the three.js forum that I've been reading through to learn more. Do you know why their ImageLoader does not do something like below where a fetch could be used to get the blob data and then load it into an img element? This is just a naive question as I've only been looking at this for a few minutes so I'm guessing there must be some deeper reason? Is this the performance issue you refer to here?

const imgurl = "https://cdn62.picsart.com/182788826000202.jpg?type=webp&to=crop&r=256"
fetch(imgurl)
  .then(response => response.blob())
  .then(myBlob => {
  var urlCreator = window.URL || window.webkitURL;
   var imageUrl = urlCreator.createObjectURL(myBlob);
   const myImgElem = document.getElementById('my-img');
   myImgElem.src = imageUrl
})

@mistic100
Copy link
Owner

mistic100 commented Sep 22, 2022

Yes that's the issue I am refering to.

@mikepianka
Copy link
Author

OK got it, thanks again. This is much more interesting than I thought it would be!

@mikepianka
Copy link
Author

Do you recall what the performance hit was like using FileLoader instead of ImageLoader? Was it for tile sets with many subdivisions of tiles or would there be a significant performance issue even for a typical pano like 16x8 512px tiled image?

I've spent some time looking for alternatives to load the images with the same authentication method and not coming up with a lot. The next viable alternative would seem to be switching to cookies but that has its own complexities.

@mistic100
Copy link
Owner

This is the Chrome profiler while loading the 24k example.

With ImageLoader :
Capture d’écran du 2022-09-23 12-29-50

With FileLoader :
Capture d’écran du 2022-09-23 12-27-45


The CPU load difference is clear (and I have a Core i7).

I could switch the loader if requestHeaders are provided but I really fear other users might complain. It is always the same problem when adding advanced settings.

@mistic100
Copy link
Owner

Let's got a runtime warning

EquirectangularTilesAdapter fallbacks to file loader because "requestHeaders" where provided. Consider removing "requestHeaders" if you experience performances issues.

@mikepianka
Copy link
Author

OK yes I do see the CPU difference. I think that it would be a good approach to enable it with file loader and provide the warning as you mentioned. This also makes it more clear than before as to why the behavior is different between adapters. I am guessing if people are fiddling with the headers it is a more advanced use case anyway where they can decide if the performance difference is acceptable for their purposes.

@mistic100 mistic100 added this to the 4.8.0 milestone Sep 23, 2022
@github-actions
Copy link

This feature/bug fix has been released in version 4.8.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants