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

Add perspective correction for image, canvas and video sources #11292

Merged
merged 3 commits into from
Nov 18, 2021

Conversation

mourner
Copy link
Member

@mourner mourner commented Nov 17, 2021

Closes #9158. Currently, our image, canvas and video sources do simple linear interpolation when mapping a texture to a quad for the layer. This only works if the quad is exactly a rectangle — in other cases, the texture is distorted:

image

This PR introduces perspective correction, calculating a perspective transform matrix given the four coordinates (based on this brilliant StackOverflow answer by @gagern 🙏), and then passing a part of it as a uniform to the raster vertex shader so that it can properly calculate the w component (used by WebGL for texture mapping correction) for each of the four vertices.

before after
image image

Launch Checklist

  • briefly describe the changes in this PR
  • include before/after visuals or gifs if this PR includes visual changes
  • write tests for all new functionality
  • document any changes to public APIs
  • post benchmark scores
  • manually test the debug page
  • tagged @mapbox/gl-native — this PR includes shader changes and needs a native port
  • apply changelog label ('bug', 'feature', 'docs', etc) or use the label 'skip changelog'
  • add an entry inside this element for inclusion in the mapbox-gl-js changelog: <changelog>Add perspective correction for non-rectangular image, canvas and video sources</changelog>

@mourner mourner requested a review from ansis November 17, 2021 15:15
@mourner mourner self-assigned this Nov 17, 2021
@mourner
Copy link
Member Author

mourner commented Nov 17, 2021

One thing I forgot to mention is that the updated render test images, apart from the perspective correction, look a little darker — not different enough to fail tests but noticeable. I guess this is some kind of color profile difference between machines / OS? The new fixtures are how the CI renderes it in Circle too so this seems fine.

@mourner mourner merged commit e7964dc into main Nov 18, 2021
@mourner mourner deleted the fix-image-perspective branch November 18, 2021 14:23
@HansBrende
Copy link

HansBrende commented Dec 14, 2021

One thing I forgot to mention is that the updated render test images, apart from the perspective correction, look a little darker

@mourner that's probably because you're not doing due diligence in correcting the image gamma before transforming the image. One of the most common bugs in most image scaling software. See: http://www.ericbrasseur.org/gamma.html

@mourner
Copy link
Member Author

mourner commented Dec 14, 2021

@HansBrende this wouldn't explain why they were lighter in fixtures before and got darker subsequently, implying we handled gamma correctly before but then somehow broke it.

@HansBrende
Copy link

HansBrende commented Dec 14, 2021

@mourner no clue! Could be that perspective transform handles gamma worse than affine transform. All I know is: when images' brightness is off after scaling/transforming... it is usually, in my experience, in fact never once has it not been, a gamma issue.

(But as the article I linked to mentions: even if gamma is handled incorrectly, the resulting image will oftentimes still look correct.)

@endanke
Copy link
Contributor

endanke commented Dec 14, 2021

@mourner I have one theory: maybe there were some other changes previously that changed the brightness, but the render tests didn't catch it because it wasn't a large enough difference. This could be verified if we revert these changes and compare the output manually with the previous expectations.

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

Successfully merging this pull request may close these issues.

Mapbox GL JS Image Overlay Coordinates Precision Issue
4 participants