MapStory / mapstory-meta

MapStory
http://mapstory.org
32 stars 16 forks source link

playback issues #670

Closed ischneider closed 11 years ago

ischneider commented 11 years ago

http://mapstory.dev.opengeo.org/maps/265/

bartvde commented 11 years ago

looking at this one now

bartvde commented 11 years ago

not an easy one again, it seems to me that there are cases where a loadend event is not fired in OpenLayers

ischneider commented 11 years ago

Share this will cause errors when run local. We should add a way to disable. On May 7, 2013 2:05 AM, "Bart van den Eijnden" notifications@github.com wrote:

this might have something to do with the frameRate, if I inspect the control it has no frameRate set. Will investigate more.

Btw getting a lot of JS errors with a debugger open, e.g. on new sharethis.widgets.hoverbutton etc.

— Reply to this email directly or view it on GitHubhttps://github.com/opengeo/mapstory/issues/670#issuecomment-17528782 .

bartvde commented 11 years ago

yeah running on 127.0.0.1 it's fine so that's good enough for me for now

bartvde commented 11 years ago

trying to reproduce this locally, but wondering why in my case the default requests don't work, I need to add T23:00:00Z to them. This is not the case on dev. Also wondering why the client is not using the hours in this case, since the GetCapabilities advertizes it correctly.

http://localhost:8080/geoserver/geonode/wms?service=WMS&version=1.1.0&request=GetMap&layers=geonode:SlateGunDeaths&styles=&bbox=-157.8285405,19.685067,-69.7908786,64.829002&width=643&height=330&srs=EPSG:4326&format=application/openlayers&TIME=2012-12-16T23:00:00Z

bartvde commented 11 years ago

Hmm and on production it does not work properly if I add T23:00:00Z, confused

http://mapstory.dev.opengeo.org/geoserver/wms?LAYERS=SlateGunDeaths&FORMAT=image%2Fpng&TRANSPARENT=TRUE&SERVICE=WMS&VERSION=1.1.1&REQUEST=GetMap&STYLES=&TILED=true&SRS=EPSG%3A900913&TIME=2012-12-14T23:00:00Z&BBOX=-10018754.17,0,-5009377.085,5009377.085&WIDTH=256&HEIGHT=256

bartvde commented 11 years ago

So but the issue here is that playback is too fast right @ischneider ? At least much faster than what it should be according to the frameRate (1 tick every second).

bartvde commented 11 years ago

The issue is somewhere here:

https://github.com/opengeo/openlayers/blob/dimNGfresh/lib/OpenLayers/Control/DimensionManager.js#L333:L355

The tick function is not called too often, but the tick event is fired too often.

bartvde commented 11 years ago

Also setCurrentValue is getting called and triggers tick, which causes the fast movement.

bartvde commented 11 years ago

gxp.slider.TimeSlider is getting into onSliderChangeComplete which is causing timeManager.setCurrentValue(timeManager.incrementTimeValue(steps)); to get called on playback

bartvde commented 11 years ago

Okay so we finally found the culprit, it's the changes to make the time slider aggressive which have caused this as a side effect.

bartvde commented 11 years ago

I'm wondering what the way out of this is though. I'm tempted to just make aggressive false for now in MapStory, but if we ever want to restore the aggressive time slider, would it be acceptable to only be aggressive when we are not playing? cc @cholmes

bartvde commented 11 years ago

Even with only listening to changes in the slider when we are not playing, the slider can "go on the run" when you move the slider in aggressive mode

cholmes commented 11 years ago

Not sure I understand your very last message.

I think agressive time slider is quite important.

I think it would be acceptable to only be aggressive while not playing. Though not ideal.

But are you saying in your last comment that that's not even possible?

bartvde commented 11 years ago

My last comment is saying that there are even issues when using aggressive time slider when not playing, i.e. the slider can go on the run. It might be related to not prebuffering in this specific case, I need to check.

cholmes commented 11 years ago

What are the cases where this arises? Is it whenever it's not doing 'use exact values list only'?

I can ask Tucker about the tradeoffs between agressive and the other option. But need to understand what cases we will do this bad playback in.

I'm also open to redoing the time slider playback with OL3 in the future. Plus maybe jquery or whatever new toolkit people want. Would a total redo let us get 'both'?

bartvde commented 11 years ago

okay I think I've found a fix that will allow aggressive when playing, it will need some testing though

bartvde commented 11 years ago

see https://github.com/opengeo/gxp/commit/36e51fbd5f8e4aec95402189d5e1a9b1af8cf17b pulled in with https://github.com/opengeo/geonode/commit/a0905552783452a37ba3dacabc34a777fff7d181

bartvde commented 11 years ago

this is ready for testing on dev

ischneider commented 11 years ago

It works better but there appear to be some odd 'stepping' issues. It seems that every 3-4 frames, one frame stays longer than the others and the next is displayed for a shorter time.

bartvde commented 11 years ago

hmm this seems to be the same playback issue as in #616 To me it seems to be related to getting errors back from the server and the client stalling on this, I've also asked @ahocevar for some feedback (my guess is that the loadend event will not be fired in the case of tile errors).

bartvde commented 11 years ago

I pulled in the latest OL (merged OL master in dimNGfresh to get the latest TileManager fixes) but that did not solve this issue

bartvde commented 11 years ago

the issue is not related to the tile manager, removing the tile manager playback also stalls on error

bartvde commented 11 years ago

Reading this: http://stackoverflow.com/questions/10125784/what-kind-of-network-error-is-chrome-encountering-when-status-failed-and could it be that the size retrieved is larger than the content-length header?

bartvde commented 11 years ago

Hmm there is no content-length header, GWC reports a miss:

HTTP/1.1 200 OK Date: Mon, 27 May 2013 15:02:40 GMT Server: Apache-Coyote/1.1 geowebcache-miss-reason: not a tile layer Expires: Mon, 27 May 2013 15:03:41 GMT geowebcache-cache-result: MISS Cache-Control: max-age=60, must-revalidate Content-Disposition: inline; filename=SlateGunDeaths.png Content-Type: image/png Via: 1.1 mapstory.dev.opengeo.org Keep-Alive: timeout=15, max=91 Connection: Keep-Alive Transfer-Encoding: chunked

bartvde commented 11 years ago

But the images that go okay have a GWC miss as well:

Cache-Control:max-age=60, must-revalidate Connection:Keep-Alive Content-Disposition:inline; filename=SlateGunDeaths.png Content-Type:image/png Date:Mon, 27 May 2013 15:02:41 GMT Expires:Mon, 27 May 2013 15:03:41 GMT geowebcache-cache-result:MISS geowebcache-miss-reason:not a tile layer Keep-Alive:timeout=15, max=91 Server:Apache-Coyote/1.1 Transfer-Encoding:chunked Via:1.1 mapstory.dev.opengeo.org

bartvde commented 11 years ago

For the failed request the timing tab of Chrome shows interesting info:

Blocking: 0 Sending: 0 Waiting: 146 ms Receiving: 15.00 s

The receiving part seems to be what the client is stalling on playback

bartvde commented 11 years ago

Maybe it's just flooding of the sockets since all go to the same domain, I'm not sure.

bartvde commented 11 years ago

Okay because the layer's url is a relative url, the code that changes to t1, t2, t3, t4 etc. does not kick in anymore, wondering when this happened though, did we change anything to relative urls?

bartvde commented 11 years ago

it's not socket flooding that is causing this unfortunately, if I let the subdomains (t1 etc) kick in, the problem is still there

bartvde commented 11 years ago

@ischneider since this happens in multiple browsers (FF, Chrome) are we sure the stalling is not caused by the server in some way? Any way to find out?

bartvde commented 11 years ago

Here is a patch that restored the subdomains (there is another duplicate set of code which needs adaption as well):

diff --git a/src/geonode-client/app/static/script/app/GeonodeViewer.js b/src/geonode-client/app/static/script/app/GeonodeViewer.js
index 0c541f5..4911558 100644
--- a/src/geonode-client/app/static/script/app/GeonodeViewer.js
+++ b/src/geonode-client/app/static/script/app/GeonodeViewer.js
@@ -58,7 +58,7 @@ var GeonodeViewer = Ext.extend(gxp.Viewer, {
      * api: config[cachedSourceMatch]
      * ``RegExp`` pattern to match the layer url to for adding extra subdomains
      */
-    cachedSourceMatch: /dev\.mapstory/,
+    cachedSourceMatch: /mapstory\.dev/,

     /**
      * api: config[cachedSubdomains]
@@ -392,15 +392,19 @@ var GeonodeViewer = Ext.extend(gxp.Viewer, {
                             singleTile: forceSingleTile,
                             transitionEffect: 'resize'
                         });
-                        if(Ext.isString(layer.url) && layer.url.search(this.cachedSourceMatch)>-1 && this.cachedSubdomains){
-                            var uparts = layer.url.split('://');
+                        var url = layer.url;
+                        if (url.indexOf('/') === 0) {
+                          url = window.location.protocol + '//' + window.location.host + url;
+                        }
+                        if(Ext.isString(url) && url.search(this.cachedSourceMatch)>-1 && this.cachedSubdomains){
+                            var uparts = url.split('://');
                             var urls = [];
                             for(var j=0, h=uparts.slice(-1)[0], len=this.cachedSubdomains.length; j<len; j++){
                                 urls.push(
                                     (uparts.length>1 ? uparts[0] + '://' : '') + this.cachedSubdomains[j] + '.' + h
                                 );
                             }
-                            layer.url = urls.concat([layer.url]);
+                            layer.url = urls.concat([url]);
                         }
                         if(layer.params) {
                             layer.params.TILED = true;

Also, the beforeconnection handler needs attention since it assumes a url is always a string, but in this case an array of strings.

cholmes commented 11 years ago

Testing this now - I guess I see the pause, but it's not super noticeable. And this is way, way better than how it worked before. So I'd be ok with pushing this as is, and investigating more later.

Though one thing I do notice in testing is that #616 fix isn't working well here - the hours are displayed, when I don't think they should be.

On Tuesday, May 28, 2013, Bart van den Eijnden wrote:

@ischneider https://github.com/ischneider since this happens in multiple browsers (FF, Chrome) are we sure the stalling is not caused by the server in some way? Any way to find out?

— Reply to this email directly or view it on GitHubhttps://github.com/opengeo/mapstory/issues/670#issuecomment-18536032 .

bartvde commented 11 years ago

then I guess something is wrong on my side, since I see a significant delay (15 seconds).

ahocevar commented 11 years ago

@bartvde, did you update OpenLayers recently? The TileManager now throttles tile loading by default, so you may want to either set tilesPerFrame to (numServers * 6) or frameDelay to 0.

bartvde commented 11 years ago

@ahocevar I did it this week but the problem is not caused by it (the problem was there before). Thanks for your pointer though, will configure it this way.

bartvde commented 11 years ago

I just made some changes to get the subdomains to work (again) and set @ahocevar's recommended setting for the TileManager, see: https://github.com/opengeo/geonode/commit/85d124a89c2d85c31bbb16ccd2d22ad083cbdd13 and https://github.com/opengeo/geonode/commit/21c464bafa8799a7cb6de4540df59029960b7e28 also deployed to dev so ready for testing

cholmes commented 11 years ago

Hmmm... The base layer seems to have gone away. I don't see it in geoserver any more. I get pink tiles on the map, and the layer page doesn't load.

ischneider commented 11 years ago

I accidentally deleted the layer during testing of my migration script. I'll reload this and verify but on my last test things were looking good.

cholmes commented 11 years ago

Yeah, it's working well enough for me. Maybe some slight pauses, but this is a minor case and we're hoping to redo the time control anyways, so not worth more time.

cholmes commented 11 years ago

this I assume got deployed, we only had the problems on dev.