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(worldview) montage requires preview size to match resolution #207

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

hkim8567
Copy link
Collaborator

@hkim8567 hkim8567 commented Nov 4, 2021

  • Update mapbox-gl version to 2.5.1 to fix white border lines that appear sometimes when rendering an image due to rounding issue. Transform is disabled to mitigate a bug introduced by 2.5.1 which results in misalignment between layers
  • viewportMinAxis is now set to minimum of resolution width and height

@@ -48,7 +48,7 @@ export const MapContainer = ({previewSize, deckProps, staticMapProps, debug, vie
resolution={resolution}
updateTimeCursor={timeMs => dispatch(updateTimeCursor(timeMs))}
debug={debug}
viewportMinAxis={viewportMinAxis}
viewportMinAxis={Math.min(resolution.width, resolution.height)}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we make this so that if the viewportMinAxis prop is provided it uses that, otherwise does this? There are situations where either is used.

@@ -175,11 +175,11 @@ export class Map extends Component {

const canvasClientSize = getCanvasClientSize(resolution, viewportMinAxis);
// canvasClientSize * scalar = previewSize
const scalar = scale(previewSize, canvasClientSize);
// const scalar = scale(previewSize, canvasClientSize);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since this only needs to be disabled for mapbox 2.5, but could still be used with 2.4 - can you treat previewSize as optional, where if it's provided then it should do this scaling and size the CSS container variable using previewSize. Otherwise (in the 2.5 case), don't scale and actually set the container size using the canvasSize variable?

@chrisgervang chrisgervang changed the title Fix montage render bug fix(worldview) montage requires preview size to match resolution Nov 7, 2021
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

Successfully merging this pull request may close these issues.

2 participants