WorldWindEarth / WorldWindJava

A community supported fork of the NASA WorldWind Java SDK (WWJ) is for building cross-platform 3D geospatial desktop applications in Java.
https://worldwind.earth/WorldWindJava/
48 stars 14 forks source link

Mapbox Tiling Problems (Possibly Since Changes to MercatorTiledImageLayer) #53

Closed robmilton closed 5 years ago

robmilton commented 5 years ago

Description

We had created a copy of MercatorTiledImageLayer from WWJ v2.1.0 with the only change being that we made the "splitScale" variable public. This allowed us to influence the "needToSplit" resulting in better looking maps from Mapbox.

We have several Mapbox layers containing raster data and several containing vector data. ALL of these layer worked with WWJ v2.1.0, but after the change in the MercatorTiledImageLayer in WWJ-CE we are having problems with the layers that contain vector data (streets, boundaries, etc.).

When the layers that contain vector data load it looks like there are missing tiles in various locations as you zoom in and out. We know the tiles aren't missing, so I'm guessing there's something about the way the tiles are being requested that's changed.

If I revert to WWJ v2.1.0 ALL of these same Mapbox layers display without any problems. I'm new to Git/GitHub, but I'm wondering if there's a way for me to revert my local copy of WWJ-CE to a point before the change to the MercatorTiledImageLayer was merged in order to confirm if this is when the issue appeared? I'll poke around and see if I can figure out how to do that.

If there's another/better way to display all our Mapbox maps in WWJ I'd love some pointers. Otherwise I need to figure out what happened and get this corrected one way or another.

Edit I figured out how to revert to just before the change to MercatorTiledImageLayer, but this did not fix the issue. If I use WWJ v2.1.0 the problem goes away. I guess I can keep reverting until I discover the point at which the problem goes away. Any thoughts on what may have caused this and how to fix it are certainly welcome.

Expected behavior: All of our Mapbox maps should load as they did while using WWJ v2.1.0.

Actual behavior: Mapbox maps containing vector data (streets, boundaries, etc.) look like they are missing tiles at various locations as the map is zoomed in and out.

Steps to Reproduce

I suppose this would be hard for anyone that doesn't use Mapbox maps. However, simply set up a map from Mapbox that contains vector data (using the OSMMapnikLayer as an example).

Reproduces how often: 100% of the time

Operating System and Version

Windows 10

Additional Information

ComBatVision commented 5 years ago

Hello. Could you provide a class which you are using to connect MapBox, so we can try to reproduce a bug. Or at least url builder.

robmilton commented 5 years ago

I've temporarily made a very simple "black" map available with a temporary access token (included in the code below). I just used the OSMMapnikLayer as a template.

If you zoom in and out particularly along the coast line around the US you'll easily observe the issue (although you can see the issue all over the globe as you zoom in and out). A great place to zoom in is around San Francisco.

Here's the class:

public class BN_Mapbox_Black extends BasicMercatorTiledImageLayer {
    static String tempToken = "pk.eyJ1IjoiYWlycG9ydG1hc3RlciIsImEiOiJjand4azM3M2MwMXMzNGFxcjZwMXR5MjJqIn0.rtQ97ju2aDx_TZ_1rl-o7g";
    static String strName = "BN_Mapbox_Black";

    public BN_Mapbox_Black() {
        super(strName, "Earth/" + strName, 20, 256, false, ".png", new URLBuilder());
    }

    private static class URLBuilder extends MercatorTileUrlBuilder {

        @Override
        protected URL getMercatorURL(int x, int y, int z) throws MalformedURLException {
            String key = "?access_token=" + tempToken;
            return new URL("https://api.mapbox.com/styles/v1/airportmaster/cjhjerqz72m0e2smwjmlzm5sm/tiles/256/" + z + "/" + x + "/" + y + key);
        }
    }

    @Override
    public String toString() {
        return strName;
    }
}

I you need it I can also provide the other classes we used (before WWJ-CE) to display these layers. As I mentioned we literally copied the MercatorTiledImageLayer in WWJ v2.1.0 created by tag and the only change we made was to make the variable "splitScale" public. We then created a copy of BasicMercatorTiledImageLayer that extends our copy of MercatorTiledImageLayer. And with WWJ-CE we had to add a copy of MercatorTextureTile from WWJ v2.1.0. Then the map layer classes look like this:

public class BN_Mapbox_BlackX extends BN_BasicMercatorTiledImageLayer {
    static String tempToken = "pk.eyJ1IjoiYWlycG9ydG1hc3RlciIsImEiOiJjand4azM3M2MwMXMzNGFxcjZwMXR5MjJqIn0.rtQ97ju2aDx_TZ_1rl-o7g";
    static String strName = "BN_Mapbox_Black";

    public BN_Mapbox_BlackX() {
        super(makeLevels());
        this.splitScale = 1.4;

        this.setPickEnabled(false);
        this.setMinActiveAltitude(500.0);
    }

    private static LevelSet makeLevels() {
        AVList params = new AVListImpl();

        int intTileWidth = 256;
        int intTileHeight = 256;
        int intNumLevels = 20;

        params.setValue(AVKey.TILE_WIDTH, intTileWidth);
        params.setValue(AVKey.TILE_HEIGHT, intTileHeight);
        params.setValue(AVKey.DATA_CACHE_NAME, "Earth/" + strName);
        params.setValue(AVKey.SERVICE, "https://api.mapbox.com/styles/v1/airportmaster/cjhjerqz72m0e2smwjmlzm5sm/tiles/256/");
        params.setValue(AVKey.DATASET_NAME, strName);
        params.setValue(AVKey.FORMAT_SUFFIX, ".png");
        params.setValue(AVKey.NUM_LEVELS, intNumLevels);
        params.setValue(AVKey.NUM_EMPTY_LEVELS, 0);
        params.setValue(AVKey.LEVEL_ZERO_TILE_DELTA, new LatLon(Angle.fromDegrees(22.5d), Angle.fromDegrees(45d)));
        params.setValue(AVKey.SECTOR, new MercatorSector(-1.0, 1.0, Angle.NEG180, Angle.POS180));
        params.setValue(AVKey.TILE_URL_BUILDER, new URLBuilder());

        return new LevelSet(params);
    }

    private static class URLBuilder implements TileUrlBuilder {

        @Override
        public URL getURL(Tile tile, String imageFormat)
                throws MalformedURLException {
            // z/x/y
            return new URL(tile.getLevel().getService()
                    + (tile.getLevelNumber() + 3) + "/" + tile.getColumn() + "/"
                    + ((1 << (tile.getLevelNumber()) + 3) - 1 - tile.getRow())
                    + "?access_token=" + tempToken);
        }

    }

    @Override
    public String toString() {
        return strName;
    }

}

Using either of the classes/methods above produces the same results.

Let me know if you need me to post the Mercator classes I described above. And thanks for any help with this!

ComBatVision commented 5 years ago

Have you checked an application log during browsing that regions? What result message of image retriever do you see?

robmilton commented 5 years ago

@Sufaev is there a log that prints the image retriever messages that I can enable/find? Or are you talking about me adding print statements somewhere?

ComBatVision commented 5 years ago

When you run application in Idea or Eclipse it shows error messages in console among other messages if image was not retrieved.

Are you able to review this log when viewing the bug area? It will give an answer if retrieval failed or it even not request required tiles.

I plan to do it myself to analyse the issue, but i have no time nearest several days to do it immediatelly.

@wcmatthysen are you able to debug? I can do it probably at the end of next week.

gbburkhardt commented 5 years ago

To see more retriever messages, run your application with JVM arg "-Djava.util.logging.config.file=logging.properties", and set ".level=FINEST" in 'logging.properties'.

For Netbeans 8.2, using the Gradle run task, I needed to set "jvm-line-args=-Djava.util.logging.config.file=logging.properties" in Project Properties|Custom Variables.

You can limit the FINEST logging by adding a line to 'logging.properties' that applies only to Worldwind: "gov.nasa.worldwind.level=FINEST".

robmilton commented 5 years ago

Thanks for the instructions. So far I haven't been able to turn on logging. I'm using NetBeans 11, and the project is an Ant project.

I added VM option -Djava.util.logging.config.file=logging.properties through the Project Properties dialog. I also tried -Djava.util.logging.config.file="logging.properties".

My project did not have a logging.properties file, so I created one under the nbproject folder. I added .level=FINEST. I also tried level=FINEST.

Any additional tips on how to get the logger working? I'll keep experimenting with it.

gbburkhardt commented 5 years ago

There is a 'logging.properties' file that's part of the repository, and my references to making changes are to it. I didn't find a place to set the JVM arguments in Netbeans 11. My opinion at the moment is that Netbeans 11 isn't ready for prime time.

It did work for me using Netbeans 8.2, the Gradle project, and setting a custom variable: image

So when running the WorldWindow sample application, one sees in the output pane: Executing: gradle :run Arguments: [-PmainClass=gov.nasa.worldwindx.applications.worldwindow.WorldWindow, -PcmdLineArgs=, -PjvmLineArgs=-Djava.util.logging.config.file=logging.properties, -c, C:\Users\glenn\Documents\repos\WorldWindJava\settings.gradle]

and then retriever messages:

Jun 16, 2019 1:55:38 PM sun.net.www.protocol.http.HttpURLConnection writeRequests FINE: sun.net.www.MessageHeader@556c39be5 pairs: {GET /wms?EXCEPTIONS=application/vnd.ogc.se_xml&REQUEST=GetCapabilities&SERVICE=WMS&VERSION=1.3.0 HTTP/1.1: null}{User-Agent: Java/1.8.0_202}{Host: worldwind25.arc.nasa.gov}{Accept: text/html, image/gif, image/jpeg, ; q=.2, /; q=.2}{Connection: keep-alive} Jun 16, 2019 1:55:38 PM sun.net.www.protocol.http.HttpURLConnection writeRequests FINE: sun.net.www.MessageHeader@284a07e65 pairs: {GET /elev?EXCEPTIONS=application/vnd.ogc.se_xml&REQUEST=GetCapabilities&SERVICE=WMS&VERSION=1.3.0 HTTP/1.1: null}{User-Agent: Java/1.8.0_202}{Host: worldwind26.arc.nasa.gov}{Accept: text/html, image/gif, image/jpeg, ; q=.2, /; q=.2}{Connection: keep-alive}

robmilton commented 5 years ago

Ok. I was able to get logging working (my path to the logging.properties file was incorrect).

Side Note Not sure if this should be taking place, but without any interaction whatsoever every 10 seconds the following messages get printed to the console:

Jun 16, 2019 3:46:02 PM sun.net.www.protocol.http.HttpURLConnection writeRequests
FINE: sun.net.www.MessageHeader@45b41a395 pairs: {GET / HTTP/1.1: null}{User-Agent: Java/1.8.0_201}{Host: worldwind.arc.nasa.gov}{Accept: text/html, image/gif, image/jpeg, *; q=.2, */*; q=.2}{Connection: keep-alive}
Jun 16, 2019 3:46:02 PM sun.net.www.protocol.http.HttpURLConnection getInputStream0
FINE: sun.net.www.MessageHeader@418b865714 pairs: {null: HTTP/1.1 200 OK}{Date: Sun, 16 Jun 2019 20:46:01 GMT}{Server: Apache}{Strict-Transport-Security: max-age=31536000;}{X-Frame-Options: sameorigin}{Last-Modified: Wed, 22 May 2019 19:30:22 GMT}{ETag: "280f-5897eff14ef80"}{Accept-Ranges: bytes}{Content-Length: 10255}{Vary: Accept-Encoding}{Access-Control-Allow-Origin: *}{Keep-Alive: timeout=5, max=100}{Connection: Keep-Alive}{Content-Type: text/html}

Map Problem When I enable the layer that is having problems and begin to zoom in I get a bunch of messages like this:

Jun 16, 2019 3:48:15 PM sun.net.www.protocol.http.HttpURLConnection writeRequests
FINE: sun.net.www.MessageHeader@2e85ffe15 pairs: {GET /styles/v1/airportmaster/<mapId>/tiles/256/7/23/46?access_token=<token> HTTP/1.1: null}{User-Agent: Java/1.8.0_201}{Host: api.mapbox.com}{Accept: text/html, image/gif, image/jpeg, *; q=.2, */*; q=.2}{Connection: keep-alive}
Jun 16, 2019 3:48:15 PM sun.net.www.protocol.http.HttpURLConnection getInputStream0
FINE: sun.net.www.MessageHeader@445e706f16 pairs: {null: HTTP/1.1 200 OK}{Content-Type: image/png}{Content-Length: 1606}{Connection: keep-alive}{Date: Sun, 16 Jun 2019 14:41:58 GMT}{X-Powered-By: Express}{Access-Control-Allow-Origin: *}{Access-Control-Allow-Methods: GET}{Cache-Control: max-age=43200,s-maxage=43200}{X-Rate-Limit-Limit: 2000}{X-Rate-Limit-Interval: 60}{ETag: W/"646-odBij72xGoJMLxxVxDHxshKsUNk"}{Age: 21976}{X-Cache: Hit from cloudfront}{Via: 1.1 db929e55bb40b085896b8e336fba2ab7.cloudfront.net (CloudFront)}{X-Amz-Cf-Id: ynP3zjp2r5wusTTcppPXu2z8-ucQ17scB71daOabf4jfQ70XcSQsFg==}
Jun 16, 2019 3:48:15 PM gov.nasa.worldwind.retrieve.HTTPRetriever doRead
FINE: Response code 200, Content length 1,606, Content type image/png, retrieving https://api.mapbox.com/styles/v1/airportmaster/<mapId>/tiles/256/7/23/46?access_token=<token>

As I zoom in to an area where I'm getting blank or missing tiles i see the following messages in the log:

Jun 16, 2019 4:02:34 PM sun.net.www.protocol.http.HttpURLConnection writeRequests
FINE: sun.net.www.MessageHeader@559fc04c5 pairs: {GET /styles/v1/airportmaster/<mapId>/tiles/256/10/158/396?access_token=<token> HTTP/1.1: null}{User-Agent: Java/1.8.0_201}{Host: api.mapbox.com}{Accept: text/html, image/gif, image/jpeg, *; q=.2, */*; q=.2}{Connection: keep-alive}
Jun 16, 2019 4:02:34 PM sun.net.www.protocol.http.HttpURLConnection getInputStream0
FINE: sun.net.www.MessageHeader@672d4c0116 pairs: {null: HTTP/1.1 200 OK}{Content-Type: image/png}{Content-Length: 103}{Connection: keep-alive}{Date: Sun, 16 Jun 2019 21:02:33 GMT}{X-Powered-By: Express}{Access-Control-Allow-Origin: *}{Access-Control-Allow-Methods: GET}{Cache-Control: max-age=43200,s-maxage=43200}{X-Rate-Limit-Limit: 2000}{X-Rate-Limit-Interval: 60}{ETag: W/"67-1JzMAbxe/4kPVm67cU7inUOVtfg"}{X-Cache: Miss from cloudfront}{Via: 1.1 fc96a34d255c927c49d016d510d83ef9.cloudfront.net (CloudFront)}{X-Amz-Cf-Pop: IAH50-C2}{X-Amz-Cf-Id: fnTPJhNAkw-L4jNAD1ST-4iyE5x42FGLH2Cltt18BzXArQ9mCLav3g==}
Jun 16, 2019 4:02:34 PM sun.net.www.protocol.http.HttpURLConnection getInputStream0
FINE: sun.net.www.MessageHeader@5a52ee2a16 pairs: {null: HTTP/1.1 200 OK}{Content-Type: image/png}{Content-Length: 103}{Connection: keep-alive}{Date: Sun, 16 Jun 2019 21:02:33 GMT}{X-Powered-By: Express}{Access-Control-Allow-Origin: *}{Access-Control-Allow-Methods: GET}{Cache-Control: max-age=43200,s-maxage=43200}{X-Rate-Limit-Limit: 2000}{X-Rate-Limit-Interval: 60}{ETag: W/"67-1JzMAbxe/4kPVm67cU7inUOVtfg"}{X-Cache: Miss from cloudfront}{Via: 1.1 93818a791c595a1ca7a7d31c28fbcb86.cloudfront.net (CloudFront)}{X-Amz-Cf-Pop: IAH50-C2}{X-Amz-Cf-Id: 385x775IhIaCFPus0cmCUXqcfDZQQXDVGMvTuyPsZqaO9_5onXbS3A==}
Jun 16, 2019 4:02:34 PM gov.nasa.worldwind.retrieve.HTTPRetriever doRead
FINE: Response code 200, Content length 103, Content type image/png, retrieving https://api.mapbox.com/styles/v1/airportmaster/<mapId>/tiles/256/10/156/393?access_token=<token>

I did a little searching about the {X-Cache: Miss from cloudfront} which seems to relate to CloudFront caching. But I don't know that this makes sense as a real issue since our other (raster) Mapbox maps load without issue.

Then after a short while I start getting messages like this:

Jun 16, 2019 4:10:00 PM gov.nasa.worldwind.retrieve.BasicRetrievalService$RetrievalExecutor beforeExecute
FINER: Cancelling request too long on the retrieval queue for https://api.mapbox.com/styles/v1/airportmaster/<mapId>/tiles/256/7/18/48?access_token=<token>
Jun 16, 2019 4:10:00 PM gov.nasa.worldwind.retrieve.BasicRetrievalService$RetrievalExecutor afterExecute
FINE: Retrieval of https://api.mapbox.com/styles/v1/airportmaster/<mapId>/tiles/256/7/21/47?access_token=<token> was cancelled

I'll continue to look through the log to see if I come across any other messages that add to the story. I appreciate any thoughts/advice/help.

ComBatVision commented 5 years ago

https://api.mapbox.com/styles/v1/airportmaster//tiles/256/7/18/48?access_token=

Are you sure that you correctly pass token and map id in code?

It looks like you view map from local cache.

robmilton commented 5 years ago

Yes, I'm sure about the token and mapId. I removed the actual mapId and token from each of the log entries I posted and replace them with <mapId> and <token> (since I was running the application using our actual map and token... not the temp map and token I included in the post above).

Again, if I switch back to WWJ v2.1.0 all of these layers load and display correctly. And the layers are identical (one of them is the " BN_Mapbox_BlackX extends BN_BasicMercatorTiledImageLayer" posted earlier in this thread). The only change I had to make in our WWJ-CE-based project was to bring back a copy of "MercatorTextureTile" from v2.1.0 that returns MercatorTextureTile arrays from the "createSubTiles" function instead of TextureTile arrays.

It looks like you view map from local cache.

I have intended to delete my map cache every time I run the application during this testing, but I may have forgotten to do so before one of these runs that I grabbed logger data from. However, I have deleted the map cache many times.

I'm confident these layers that retrieve Mapbox maps haven't changed. And if I use WWJ v2.1.0 they all load/display correctly. So everything point to some change between WWJ v2.1.0 and WWJ-CE that is causing this behavior.

robmilton commented 5 years ago

So far I haven't seen anything else in the log that is helpful. I don't understand why the other Mapbox layers still load/display without problem and why ALL the Mapbox layers load/display without problems if I go back to WWJ v2.1.0.

wcmatthysen commented 5 years ago

@robmilton, we can roll this change back and keep it on a separate branch until it is stable. @Sufaev, I don't know if that is ok for now?

robmilton commented 5 years ago

@wcmatthysen, I'm not 100% sure that the MercatorTiledImageLayer change is the issue. I'm new to Git, but I believe I reset my clone of WWJ-CE to just before that change was merged. But when I rebuilt WW and placed the new jar in my project I still had the same problems. So possibly an earlier change in WWJ-CE is the actual cause?

I don't know if I should continue to reset my copy of WWJ-CE at various points further back to try to discover which merge coincides with the problem?

EDIT I just looked at resetting to an earlier commit when I realized that I had reset to "563f37f - Merge pull request #35 from wcmatthysen /macos-pixel-scale-fix" which was the commit just before "a840f19 - Merge pull request #41 from Sufaev /feature/refactor-mercator-layer". However, as I look down the list of commits I see earlier commits that have to do with the Mercator layer.

So it does seem possible that the MercatorTiledImageLayer change is the issue. I will try to reset to a point prior to any commits related to the Mercator layer change. Although I'm not entirely sure how to identify a good point to try. Any suggestions?

robmilton commented 5 years ago

Ok. I reset my WWJ-CE project to commit "3399c8a - Update README.md" and the problem went away. So unless you feel like there may be another commit/merge in there that could be responsible for this issue it seems that rolling back the change to MercatorTiledImageLayer may resolve this.

ComBatVision commented 5 years ago

Am I correctly understand that reverting MercatorTiledImageLayer does not fix issue, and the problem is somewhere between 2.1.0 and MercatorTiledImageLayer enhancement?

robmilton commented 5 years ago

I'm not sure how to answer that. I reverted to the commit just before the MercatorTiledImageLayer merge and the problem still existed. So then I reverted to commit "3399c8a" and the problem went away.

So I'm not sure where I would need to revert to in order to eliminate the MercatorTiledImageLayer change as the problem. What I do know is that reverting to commit "3399c8a" caused the problem to go away.

So does that mean that the change to MercatorTiledImageLayer is not the issue? If you provide me with a commit to revert to that eliminates the possibility of the change to MercatorTiledImageLayer being the issue then I'll give that a try as well.

robmilton commented 5 years ago

@Sufaev, what commit would be the oldest/earliest that was related to the MercatorTiledImageLayer changes? Is "9f17925" the oldest/first commit that is associated with this change? If so should I just revert to "1855da5"? Would this tell us if the issue lies in the MercatorTiledImageLayer changes?

Or is there another method I can use to "roll back" the changes to the MercatorTiledImageLayer merge other than reverting the project to a specific commit?

ComBatVision commented 5 years ago

I will try to debug tomorrow. By the way, could you record some short video or make a screenshot of a problem?

robmilton commented 5 years ago

I've reverted as far back as "86454FA" but still have the problem. I'll keep working on this tomorrow, but I've got meetings I have to attend now.

I've attached several screenshots showing the problem and what the globe/layer should look like. If I continue to zoom in the positions of the missing tiles change but you continue to have many missing tiles at all zoom levels.

BAD 1 bad1

GOOD 1 good1

BAD 2 bad2

GOOD 2 good2

wcmatthysen commented 5 years ago

@robmilton, I've tried to follow what you did and it seems to be the upgrade to JOGL 2.3.2. You mentioned that you checked out 3399c8a and you had no problems. That commit is just before the JOGL upgrade. The 86454fa commit is just after the JOGL upgrade where you seem to have the abovementioned problems. Can you just double-check that this is the case?

robmilton commented 5 years ago

@wcmatthysen, I think you're right. Just to make sure I reverted to 010ef7f and the problem went away. So then I reverted to 3a1f84d and the problem appeared. It seems like the upgrade to JOGL 2.3.2 triggered the issue. Is there anything that stands out as an obvious reason to you?

I've got more meetings I have to participate in, but as soon as I get a chance I'm going to dig through any JOGL related code changes that might have impacted the Mercator classes (and/or classes they extend).

wcmatthysen commented 5 years ago

@robmilton, I'm not sure how the JOGL upgrade causes this issue. We basically just switched to the latest JOGL jars and updated a couple of imports in the WorldWind code base as the latest version of JOGL changed package names. However, what I suggest is that you perhaps first try to upgrade your graphics card driver to the latest version and see if that fixes the issue. Otherwise we'll have to dig through the code to determine the cause.

robmilton commented 5 years ago

@wcmatthysen, I checked my drivers, and I don't think there's an issue with them. My laptop has dual video cards. The NVidia driver is the latest from NVidia. The Intel driver is the latest from Dell (not the latest from Intel, but I'd rather stick with the driver Dell has released unless there's a specific reason to modify). Below are the details...

  NVIDIA GeForce GTX 980M   Driver Date: 5/22/2019   Driver Ver: 26.21.14.3086  OpenGL Ver: 4.6
  Intel HD Graphics 530     Driver Date: 2/28/2018   Driver Ver: 23.20.16.4973  OpenGL Ver: 4.5

I am able to select which video card is used when launching an application. I have used both video cards, and I get the same results with both (the same problem exists with both).

wcmatthysen commented 5 years ago

@robmilton, maybe just test on another machine if you can to make sure that it is not graphics card related.

ComBatVision commented 5 years ago

Sorry, that I still did not join debugging of this problem (very busy last few days at work), but I have several ideas to check: 1) Have you tried to copy URL, produced by new URLBuilder which is based on new MercatorURLBuilder, when you begin to experience a bug, and paste it to browser to check if URL is correctly open an image? If not, than the problem is in URL preparation. 2) Have you checked if image retiever successfully download this image and parse this url to BufferedImage and then reproject it by post downloading process without exeptions? If not, it can be CORS problem with https etc. 3) Looking to your screenshots it seems that system skip tiles which are completely filled with one grey color and displays those which have some content, is it true or I mistake?

I do not think that new code can download and display one tiles and do not download another - the reason is somewhere in filtering conditions, for example tiles may be skipped if they reach maximum allowed level of details, based on distance to tile (i saw your old code has override level of details), or tiles can be skipped if skipBlanls flag is set and then completely transparent tiles are skipped, or you set not enough number of levels.

I am trying to do my best to look at this problem myself also.

robmilton commented 5 years ago

@wcmatthysen, the issue is happening on many computers. Each of my managers and several customers have reported the problem. We have enough other maps available that I can hold off rolling back a little longer. But this appears to be happening on every machine that runs the application.

robmilton commented 5 years ago

In addition to the tiling issue I've noticed that the color of the maps (for those that contain raster data) is quite different. Below is a shot of the original map as it appears straight from Mapbox (and it looks this way if I revert to before the JOGL change) followed by a shot of the way the map looks now.

Correct Colors osmgood1

After JOGL Change osmbad1

robmilton commented 5 years ago

@Sufaev, thanks for the thoughts about troubleshooting.

Have you tried to copy URL, produced by new URLBuilder which is based on new MercatorURLBuilder, when you begin to experience a bug, and paste it to browser to check if URL is correctly open an image? If not, than the problem is in URL preparation.

  • I've attempted to do this. It's hard to determine which tiles are being displayed "correctly" and which aren't, and I'm not sure how to figure out the URL for a specific location on the map. However, every URL I've grabbed from the log has returned a tile.

Have you checked if image retiever successfully download this image and parse this url to BufferedImage and then reproject it by post downloading process without exeptions? If not, it can be CORS problem with https etc.

  • I'm not sure how to perform the steps in number 2. I'll have to dig into this. However, it would seem strange to me that a CORS issue would affect just some of the tiles.

Looking to your screenshots it seems that system skip tiles which are completely filled with one grey color and displays those which have some content, is it true or I mistake?

  • I don't believe that's true 100% of the time. I need to do some more testing, but I believe I have tiles not displaying correctly that should have some content drawn in them. I'll have to get back to you on that.
robmilton commented 5 years ago

Looking to your screenshots it seems that system skip tiles which are completely filled with one grey color and displays those which have some content, is it true or I mistake?

  • It does look this way from the screen captures posted so far. But at different zoom levels and different locations there are definitely tiles with content that don't display correctly (see below).

This is interesting. The screen capture below shows the map at zoom level 10 (according to the "z" value being requested in the URL. Notice the tile just north of the San Francisco airport is blank/black (I added the red square around the tile I'm talking about). mapmissing

By experimenting with the URL I was able to discover that the tile that should appear just north of the San Francisco airport is found here and looks like this: missingtile

After finding this tile to focus on I killed the application, cleared my map cache, then relaunched the app. I see two references to this tile in the logs (see below), but the tile never displays. Here are the two log entries that reference this tile:

Jun 19, 2019 5:07:09 PM sun.net.www.protocol.http.HttpURLConnection writeRequests
FINE: sun.net.www.MessageHeader@27b53f795 pairs: {GET /styles/v1/airportmaster/cjhjerqz72m0e2smwjmlzm5sm/tiles/256/10/163/395?access_token=pk.eyJ1IjoiYWlycG9ydG1hc3RlciIsImEiOiJjand4azM3M2MwMXMzNGFxcjZwMXR5MjJqIn0.rtQ97ju2aDx_TZ_1rl-o7g HTTP/1.1: null}{User-Agent: Java/1.8.0_201}{Host: api.mapbox.com}{Accept: text/html, image/gif, image/jpeg, *; q=.2, */*; q=.2}{Connection: keep-alive}
Jun 19, 2019 5:07:09 PM gov.nasa.worldwind.retrieve.HTTPRetriever doRead
FINE: Response code 200, Content length 2,176, Content type image/png, retrieving https://api.mapbox.com/styles/v1/airportmaster/cjhjerqz72m0e2smwjmlzm5sm/tiles/256/10/163/395?access_token=pk.eyJ1IjoiYWlycG9ydG1hc3RlciIsImEiOiJjand4azM3M2MwMXMzNGFxcjZwMXR5MjJqIn0.rtQ97ju2aDx_TZ_1rl-o7g

Have you checked if image retiever successfully download this image and parse this url to BufferedImage and then reproject it by post downloading process without exeptions? If not, it can be CORS problem with https etc.

  • It seems that the "Response code 200" in the log entry would rule out a CORS issue, don't you think?

It's beyond me at this point how a JOGL upgrade is tied to this. I'll keep digging.

wcmatthysen commented 5 years ago

@robmilton, I'm seeing a similar issue. I created a MapboxLayer class for testing purposes and signed up for a mapbox account to test (using the free-tier). The class I'm using is:

package gov.nasa.worldwind.layers.Earth;

import gov.nasa.worldwind.layers.mercator.BasicMercatorTiledImageLayer;
import gov.nasa.worldwind.layers.mercator.MercatorTileUrlBuilder;

import java.net.MalformedURLException;
import java.net.URL;

public class MapboxLayer extends BasicMercatorTiledImageLayer {

    public MapboxLayer()
    {
        super("mb", "Earth/Mapbox", 19, 256, false, ".png", new URLBuilder());
    }

    private static class URLBuilder extends MercatorTileUrlBuilder
    {
        private String accessToken;

        @Override
        protected URL getMercatorURL(int x, int y, int z) throws MalformedURLException
        {
            String urlPostfix = (this.accessToken != null) ? "?access_token=" + this.accessToken : "";
            return new URL("https://api.mapbox.com/styles/v1/mapbox/streets-v11/tiles/256/" + z + "/" + x + "/" + y + urlPostfix);
        }
    }

    public void setAccessToken(String apiKey)
    {
        URLBuilder urlBuilder = (URLBuilder)getURLBuilder();
        urlBuilder.accessToken = apiKey;
    }

    public String getAccessToken()
    {
        URLBuilder urlBuilder = (URLBuilder)getURLBuilder();
        return urlBuilder.accessToken;
    }

    @Override
    public String toString()
    {
        return "Mapbox";
    }
}

I then added the MapboxLayer to worldwind.layers.xml as follows:

<Layer className="gov.nasa.worldwind.layers.Earth.MapboxLayer" actuate="onRequest">
    <Property name="AccessToken" value="<access-token-here>"/>
</Layer>

replacing <access-token-here> with the access token I got from mapbox. I then ran the gov.nasa.worldwindx.examples.LayerTreeUsage example. The results I got was similar to yours with missing white tiles. However, I then manually reverted my version of JOGL back to how it was originally by getting the original jars from the upstream repository and copying them to the WorldWind-CE folder, changing the package names back to how they were (i.e. replacing org.jogamp.opengl with javax.media.opengl etc.) and rebuilding WorldWind against the old JOGL. The funny thing is, that I'm still getting the issue. Here is the screenshot:

Screenshot from 2019-06-20 10-59-10

I also made sure to clear my tile-cache on every run. So, I'm not exactly sure that this is a JOGL issue.

wcmatthysen commented 5 years ago

@robmilton, if you want you can check out my mapbox-test branch. It contains the MapboxLayer class as well as the changes to revert to the previous version of JOGL. I just had to build the project with ant as the gradle build messed up in NetBeans for me.

wcmatthysen commented 5 years ago

@robmilton, I looked in my tile-cache directory and it seems that there are white tiles in there. For example, in the Earth/Mapbox/0/3 folder I see:

tiles

This is similar for deeper levels if you look through the tile-cache. So, it seems that these tiles are fetched and processed from mapbox and the resulting white tiles are stored on disk. Somewhere between fetching, processing, and storing these tiles, something goes wrong.

robmilton commented 5 years ago

@wcmatthysen, that's very interesting and confusing. I checked out your branch and saw exactly what you're describing.

I thought maybe there would be a difference if I used the classes we're using to define the layer (older WW classes we extended in order to be able to set the "splitScale" value). So I added those classes and created two layers: Mapbox_Black and Mapbox_Streets_OSMStyle. All of the classes I added are in the gov.nasa.worldwind.layers.Earth package. For simplicity I included a temporary token in the layer classes themselves.

However, I got the same results with these layers and classes as you got with the new BasicMercatorTiledImageLayer class. There must be something else that changes when I revert to just before the JOGL merge. I'll keep digging.

I created a repository called mapboxtest you can check out if you're interested in seeing the other classes we're using.

robmilton commented 5 years ago

@wcmatthysen, I'm seeing the same thing...

In my Mapbox_Streets_OSMStyle cache I get white tiles that I don't believe should be white: white

In Mapbox_Black I get black tiles that I don't believe should be black: black

And in the Mapbox_Streets_OSMStyle map the colors are wrong (as I mentioned in a post above): color

robmilton commented 5 years ago

Ok, I finally figured out the issue. When I would revert to earlier commits in the WWJ-CE project I also had to revert to earlier commits in our project. There was a single line in one class that had been changed that I didn't find until literally comparing line-by-line.

As I mentioned above we had created a copy of the "MercatorTiledImageLayer" from WWJ v2.1.0 so that we could expose the "splitScale" variable. What I didn't realize was that there was an additional change that hadn't been documented in our "BasicMercatorTiledImageLayer" class that extends the "MercatorTiledImageLayer" class.

Inside "BasicMercatorTiledImageLayer" is a function called "transform". Inside the function the WWJ v2.1.0 code retrieved the BufferedImage type, and if the type was equal to 0 it set the type equal to "BufferedImage.TYPE_INT_RGB".

Someone must have figured out that when working with the Mapbox maps this value needed to be "BufferedImage.TYPE_INT_ARGB". Setting the type to ARGB fixes the blank tiles and coloring issues seen above. Below is the function with the changes described:

    private BufferedImage transform(BufferedImage image, MercatorSector sector) {

        // *********************************************************************
//        // Old
//        int type = image.getType();
//        if (type == 0) type = BufferedImage.TYPE_INT_RGB;

        // New
        int type = BufferedImage.TYPE_INT_ARGB;
        // *********************************************************************

        BufferedImage trans = new BufferedImage(image.getWidth(), image.getHeight(), type);
        double miny = sector.getMinLatPercent();
        double maxy = sector.getMaxLatPercent();
        for (int y = 0; y < image.getHeight(); y++) {
            double sy = 1.0 - y / (double) (image.getHeight() - 1);
            Angle lat = Angle.fromRadians(sy * sector.getDeltaLatRadians() + sector.getMinLatitude().radians);
            double dy = 1.0 - (MercatorSector.gudermannianInverse(lat) - miny) / (maxy - miny);
            dy = Math.max(0.0, Math.min(1.0, dy));
            int iy = (int) (dy * (image.getHeight() - 1));
            for (int x = 0; x < image.getWidth(); x++) {
                trans.setRGB(x, y, image.getRGB(x, iy));
            }
        }
        return trans;
    }

The current "BasicMercatorTiledImageLayer" class in WWJ-CE contains a class called "MercatorDownloadPostProcessor" which has a method called "transformPixels" which calls "transformPixels" in the "AbstractRetrievalPostProcessor" class then the rest of the code is almost identical to the old "transform" function.

Would it be beneficial to make some modifications so that the new Mercator classes will work with Mapbox maps? I'm not exactly sure what that would entail, but I'd be glad to work on it as I have opportunity to do so.

ComBatVision commented 5 years ago

I have already fixed related issue earlier and have added additional check:

if (type == BufferedImage.TYPE_CUSTOM) type = BufferedImage.TYPE_INT_RGB;
else if (type == BufferedImage.TYPE_BYTE_INDEXED) type = BufferedImage.TYPE_INT_ARGB;

In my implementation I have replaced 0 with BufferedImage.TYPE_CUSTOM constant. I leave TYPE_INT_RGB for TYPE_CUSTOM just because it was the same logic in original code.

Are you sure that other maps will not fail if they will be loaded with 24 bit color space by default?

robmilton commented 5 years ago

I see that the code you mentioned is currently in the "transformPixels" function in the "MercatorDownloadPostProcessor" class in "BasicMercatorTiledImageLayer". However, when we use that code to load the Mapbox maps this problem still exists (see @wcmatthysen 's mapbox-test branch he posted yesterday).

I'm guessing that the else if (type == BufferedImage.TYPE_BYTE_INDEXED) test is not catching the Mapbox image tiles for some reason.

Are you sure that other maps will not fail if they will be loaded with 24 bit color space by default?

  • No, I'm not sure. That's what I meant when I said I'm not exactly sure what it would entail to incorporate the "fix". Maybe there is a case that can be caught in the "if" statement you referenced. Or maybe we include a custom TiledImageLayer class that handles these Mapbox maps.
ComBatVision commented 5 years ago

Does PR #54 fixed current issue?

gbburkhardt commented 5 years ago

I think it's unlikely that PR #54 will fix this issue, but I haven't tried, nor have I had time to look into the Mapbox issue. The bug #54 fixes has to do with files read in by GDAL that have no alpha band. The Mapbox issue doesn't sound anything like that.

robmilton commented 5 years ago

I have not tested #54, but I also don't think it will address this issue. I am not using a newer version of GDAL at this point. Instead I copied the function and class from the old GDALUtils that allows me to load the version of GDAL that was included with WWJ v2.1.0.

As I mentioned above I believe the Mapbox tiles are not meeting the condition in the else if (type == BufferedImage.TYPE_BYTE_INDEXED) statement in the "transformPixels" function for some reason and so the BufferedImage.TYPE_INT_ARGB is not being set. However, I'm having a hard time using the debugger on the WW project when running the "LayerTreeUsage" example. So I haven't been able to verify this yet.

robmilton commented 5 years ago

I think I discovered the issue. I'm not sure exactly why, but some of the Mapbox tiles are not being caught in the statement else if (type == BufferedImage.TYPE_BYTE_INDEXED) in "transformPixels". I discovered that some of the tiles report a "type" of BufferedImage.TYPE_BYTE_BINARY. So I modified the statement to be else if (type == BufferedImage.TYPE_BYTE_INDEXED || type == BufferedImage.TYPE_BYTE_BINARY), and the Mapbox maps seem to load properly.

There is a secondary issue with the Mapbox maps that I mentioned earlier. We created a copy of the old "MercatorTiledImageLayer" so that we could expose the "splitScale" variable. This gave us the ability to influence the "needToSplit" function to improve the appearance of the Mapbox tiles.

I've modified a branch and placed it here if you'd like to take a look. If you run the example "LayerTreeUsage" you'll see layers called "Mapbox_Black", "Mapbox_Streets_OSMStyle", "Mapbox_Black_2" and "Mapbox_Streets_OSMStyle_2". All of these layers should load and color tiles correctly now. The layers with the suffix "_2" use the new "MercatorTiledImageLayer", while the other two layers use the class "Mapbox_BasicMercatorTiledImageLayer". If you zoom in fairly close on the "Mapbox_Streets_OSMStyle_2" layer you'll notice that the tiles look a little blurred. Then if you switch to "Mapbox_Streets_OSMStyle" (without the "_2" suffix) the appearance of the map is better because of the "splitScale" value being modified.

Could the new "MercatorTiledImageLayer" be modified to provide influence over the "needToSplit" functionality in a similar fashion?

ComBatVision commented 5 years ago

On my opinion it will be usefull to make needToSplit controllable from the box. @wcmatthysen what do you think?

By the way, type of BufferedImage in transformation logic should influence only the availability of transparent pixels on resulted image. Incorrect value may cause opaque overlays. But I do not understand how it can influence current bug appearance. The only idea of why bug appears is that creation new BufferedImage of type BINARY is not supported and lead to some wrong result.

wcmatthysen commented 5 years ago

@Sufaev, we can definitely add functionality to control the needToSplit function via a splitScale variable. I think it will be best to add this, together with the bugfix of @robmilton into a new pull request that we can then merge into develop.

gbburkhardt commented 5 years ago

@wcmatthysen, I don't see the change for BufferedImage.TYPE_BYTE_BINARY fixing the problem demonstrated in your test branch, whether JOGL 2.3.2 is used or the older version is used. I was able to duplicate the problem using @robmilton's test branch, and there, removing the code for BufferedImage.TYPE_BYTE_BINARY breaks the image.

I was never able to see any wrong color display as described in @robmilton's earlier comment.

The 'OpenTopoMap' layer that's commented out in 'worldwind.layers.xml' has tiles that come in as TYPE_BYTE_BINARY as well. Adding @robmilton's change to the standard code gets rid of large white sections in the Atlantic. The layers OSMMapnikLayer, OSMCycleMapLayer, WikimapiaLayer also use the BasicMercatorTiledImageLayer class, but the servers for OSMMapnikLayer, OSMCycleMapLayer are down or require an API key, and WikimapiaLayer doesn't appear to have any TYPE_BYTE_BINARY tiles.

The change can be made to set the BufferedImage type to TYPE_INT_RGB, and it fixes the problem. That might use a bit less memory.

I don't know enough about this type of data to say what the correct solution is.

robmilton commented 5 years ago

@gbburkhardt, we are hard-coding int type = BufferedImage.TYPE_INT_ARGB; to address the problem on our end (that's working with a class that extends the OLD (WWJ v2.1.0) BasicMercatorTiledImageLayer class). If we set the type to TYPE_INT_RGB it does not fix the problem with the Mapbox maps. Having the type set to TYPE_INT_RGB seems to be the cause of the problem. And setting the type to TYPE_INT_ARGB is what corrected the issue.

gbburkhardt commented 5 years ago

I thought that the problem was when a tile came in with TYPE_BYTE_BINARY (12), that the type used to create the new BufferedImage would be the same as that of the incoming image.

In the 2.1.0 NASA code, this test was done:

if (type == 0)
        type = BufferedImage.TYPE_INT_RGB;

@Sufaev changed this in commit 9f179252 to

int type = image.getType();
if (type == BufferedImage.TYPE_CUSTOM) type = BufferedImage.TYPE_INT_RGB;
else if (type == BufferedImage.TYPE_BYTE_INDEXED) type = BufferedImage.TYPE_INT_ARGB;

So if the incoming type is TYPE_BYTE_BINARY (12), in both cases, the type will be unchanged. Your change made the type of the new buffer to be TYPE_INT_ARGB, which includes an alpha band. I determined by experiment that the code could also be fixed by

if (type == BufferedImage.TYPE_CUSTOM) type = BufferedImage.TYPE_INT_RGB;
else if (type == BufferedImage.TYPE_BYTE_INDEXED) type = BufferedImage.TYPE_INT_ARGB;
else if (type == BufferedImage.TYPE_BYTE_BINARY) type = BufferedImage.TYPE_INT_RGB;

using your test branch.

wcmatthysen commented 5 years ago

@gbburkhardt, the changes in my mapbox-test branch only demonstrated that the problem was still present when reverting back to the old JOGL. We tried to determine whether reverting to the old version of JOGL would fix the problem, and my branch demonstrated that it didn't have anything to do with the JOGL version.

It looks like you guys narrowed the problem down. I think we need to create a pull-request with this code to fix the bug (as mentioned by @gbburkhardt):

if (type == BufferedImage.TYPE_CUSTOM) type = BufferedImage.TYPE_INT_RGB;
else if (type == BufferedImage.TYPE_BYTE_INDEXED) type = BufferedImage.TYPE_INT_ARGB;
else if (type == BufferedImage.TYPE_BYTE_BINARY) type = BufferedImage.TYPE_INT_RGB;

@robmilton, would you be able to create a pull-request for this? @gbburkhardt or I could help with this. We should add the ability to control the splitScale variable as mentioned earlier to avoid blurry maps.

robmilton commented 5 years ago

@wcmatthysen, I'd be glad to. I'm not sure how to address the "splitScale" variable and "needToSplit" function, though, since the "splitScale" variable only existed in the WWJ v2.1.0 version of MercatorTiledImageLayer... and the "needToSplit" function in that class is very different than the "needToSplit" function in the TiledImageLayer class.

I've experimented with using the "detailHint" variable to accomplish the same thing, but that hasn't worked. I've also experimented with adding a "splitScale" variable and subtracting the value from the "scaledEyeDistanceMeters" value on line 520 of TiledImageLayer, but that hasn't worked either.

Any idea how to add a variable that gives the same ability to impact the "needToSplit" function as the "splitScale" variable provided in the WWJ v2.1.0 version of MercatorTiledImageLayer?

wcmatthysen commented 5 years ago

I'm not too familiar with those classes. I'll have to take a look at it. @Sufaev, would you be able to help with this?