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 the sub-pixel white grid-like image border issue in Chrome. #96

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

Conversation

lmytime
Copy link
Contributor

@lmytime lmytime commented Nov 20, 2024

This Pull Request addresses the issue of sub-pixel white borders in Chrome as mentioned in Issue #95

Fixes #95

@ryanhausen
Copy link
Owner

@lmytime Thanks your work on this. Do you know if this would fix leaflet more generally? This is a known issue in leaflet I think Leaflet/Leaflet#3575.

If this would fix leaflet in general, it might be better to submit this to leaflet, and the get the fix through them.

@lmytime
Copy link
Contributor Author

lmytime commented Nov 22, 2024

Thanks for this suggestion @ryanhausen . I think it would be hard to fix leaflet more generally. My solution only works for a given condition of 100% zoom level. But I would test some suggestions mentioned in that leaflet issue later.
Also, to be honest, it could be hard to get fixed from leaflet. That issue began nearly ten years ago...
So if you like, it would be better if we can (temporarily) fix it from our sides, using an acceptable approach.

@ryanhausen
Copy link
Owner

@lmytime can you share the map that you get the subpixel grid on. The one in #95 would be great. I don't see it all the time for myself, even with Chrome.

@lmytime
Copy link
Contributor Author

lmytime commented Nov 23, 2024

@ryanhausen I see the grid on any Fitsmap using Chorme. But on Safari, there is no gird. That screenshot in #95 is the JADES map: https://jades-survey.github.io/viewer/

@ryanhausen
Copy link
Owner

@lmytime Oh i see. I think this might be specific to Mac. I don't see it in chrome on my linux and windows machines, but I found a mac laptop and do indeed see the gridding.

I'll review and test this week. I want to make sure it won't negatively affect windows and linux

Copy link
Owner

@ryanhausen ryanhausen left a comment

Choose a reason for hiding this comment

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

@lmytime thanks again for taking the time to track down a fix for this! I took a look at the code and there were a couple issues I think should be addressed before we can merge into main.

I hacked together some fixes on my local to get it running to test the functionality for any issues, and didn't find any.

After you get the fixes in I think we can merge to main and see if any issues arise for other users.

// Convert the matched x, y, and z values to floating point numbers, then round them to the nearest integer.
var xTranslateInt = Math.round(parseFloat(matches[1])); // Round the x component to the nearest integer
var yTranslateInt = Math.round(parseFloat(matches[3])); // Round the y component to the nearest integer
var zTranslateInt = Math.round(parseFloat(matches[5])); // Round the z component to the nearest integer (typically 0)
Copy link
Owner

Choose a reason for hiding this comment

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

This currently doesn't run as is, matches here should be xyzMatches I think.


// Function to round the x and y components of the `translate3d` CSS transformation to integer values.
// This function is executed after the map ends a movement or zoom operation.
const integerTranslateMapPane = function () {
Copy link
Owner

Choose a reason for hiding this comment

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

rather than rely on the global map variable it might be better to get the map object from the event, this would be robust to changes in the names of variables in index.js Change to something like:

const integerTranslateMapPane = function (event) {
    // Obtain the map pane element, which contains the current translation transformation information.
    var mapPane = event.target.getPane('mapPane');

Copy link
Owner

Choose a reason for hiding this comment

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

A minified version of this file needs to be created called integerTranslate.min.js . The index.js js file listing should be updated to reflect the minifed file name as well. The minified version will reduce file size transfer and the cartographer only copies minfied js files into the js directory, so this file doesn't currently get moved at all and the site errors out.

The way I make minified js files is by pasting the file contents into the following website, https://minify-js.com/ then pasting the output into the appropriately named file.

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.

Map has sub-pixel white grid-like image border in Chrome
2 participants