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

Pieces flickers on update #52

Open
sesse opened this issue Nov 21, 2013 · 28 comments
Open

Pieces flickers on update #52

sesse opened this issue Nov 21, 2013 · 28 comments

Comments

@sesse
Copy link

sesse commented Nov 21, 2013

Hi,

When I do board.position(fen), it would seem the pieces are somehow redrawn; at least they flicker ever so shortly during the move somehow. Especially for the pieces that stand still, maybe this could avoided?

I don't think this has anything to do with my code; it happens on e.g. http://chessboardjs.com/examples#3001 when I switch positions.

I'm using Chrome 31.0.1650.57 on Linux.

@bs
Copy link

bs commented Nov 21, 2013

I'm experiencing the same, but haven't had a chance to dive into it yet.

Interestingly, the examples on http://chessboardjs.com seem to work fine, but my implementation flickers. The only difference is that I have specified paths to the assets. Going to try to find some time to research this further within the next week.

Doesn't flicker in Firefox, only Chrome and Safari.

@oakmac
Copy link
Owner

oakmac commented Nov 23, 2013

@sesse - can you make a video of this happening?

@sesse
Copy link
Author

sesse commented Nov 23, 2013

Sure; see http://storage.sesse.net/chessboardjs-flicker.avi. You can see it flickers a few times there (although not every time).

@bs
Copy link

bs commented Nov 23, 2013

Did a big of debugging.

The really odd bit is that I can reproduce it locally, but don't see it at all on the examples on chessboardjs.com. Just tried running it through a Node Express server in case Chrome's file:// was at fault, but still flickers.

Here's my small test code: https://github.com/bs/jschesstest
And here's a video of it in action: https://vimeo.com/80140179

@oakmac, can you try running that code locally and see if you can reproduce?

I've chased it down to the drawPositionInstant method when called from doAnimations.

@Zolmeister
Copy link

I may have found a solution, see #55

@bs
Copy link

bs commented Nov 24, 2013

@Zolmeister Doesn't fix.

@bs
Copy link

bs commented Nov 24, 2013

This seems to be a Node/Express issue for me. Running my test via python -m SimpleHTTPServer is silky smooth.

@sesse
Copy link
Author

sesse commented Nov 24, 2013

My guess is that the entire thing with the web server (be it Node.js, local serving or whatever) is pretty much incidental; there are no HTTP requests going on during a move, even though you could maybe imagine something odd with different HTTP headers. A JavaScript library shouldn't be flicker no matter what server originally served its assets anyway, though.

@Zolmeister
Copy link

I was most definitely recording an HTTP request during an animation, what may be happening is the 'Expires' cache header may not be set for the basic Express server.

Here on line 654:

function buildPiece(piece, hidden, id) {
  var html = '<img src="' + buildPieceImgSrc(piece) + '" ';
  if (id && typeof id === 'string') {
    html += 'id="' + id + '" ';
  }
  html += 'alt="" ' +
  'class="' + CSS.piece + '" ' +
  'data-piece="' + piece + '" ' +
  'style="width: ' + SQUARE_SIZE + 'px;' +
  'height: ' + SQUARE_SIZE + 'px;';
  if (hidden === true) {
    html += 'display:none;';
  }
  html += '" />';

  return html;
}

Is being called here, and here

@bs
Copy link

bs commented Nov 25, 2013

Setting the Expires header fixed it in Express for me.

Still, is proper request caching needed here? The buildPiece method is being called once for every piece on the board whenever a move occurs. Is there a reason each HTML chunk isn't build once and cached?

@oakmac
Copy link
Owner

oakmac commented Nov 29, 2013

The webserver sending appropriate expires headers and returning HTTP 304 Not Modified is probably the solution here (at least for now).

I might change the way this works in the future.

@oakmac
Copy link
Owner

oakmac commented Nov 29, 2013

@Zolmeister
Copy link

Even with HTTP 304 header you still get flickering (which is why I implemented #55).

@sesse
Copy link
Author

sesse commented Nov 29, 2013

Hi,

The solution in #55 still flickers for me (judging from http://queens.zolmeister.com/), just as much as the normal chessboard.js does. And there is not a single HTTP request going on, so really, all this talk about Expires and 304 is not the full story.

@Zolmeister
Copy link

Indeed, but I think #55 highlights the root of the problem, which is that the way animations are written, a new piece is generated and added (i.e. a new element) which seems to me like the most likely culprit.

@sesse
Copy link
Author

sesse commented Nov 29, 2013

On Fri, Nov 29, 2013 at 02:01:04AM -0800, Zoli Kahan wrote:

Indeed, but I think #55 highlights the root of the problem, which is that
the way animations are written, a new piece is generated and added (i.e. a
new element) which seems to me like the most likely culprit.

That sounds much more likely, yes. If possible, elements should
be reused (I don't know the best strategy for introducing new pieces on the
board, but that should be much more rare).

/* Steinar */

Homepage: http://www.sesse.net/

@ingararntzen
Copy link

it flickers in chrome, but not in Firefox as far as I can see.

(I haven't implemented #55)

I'd be keen to have an update that works in chrome too :)

Cheers,

Ingar Arntzen

@ingararntzen
Copy link

Hi - I have fixed the problem by reprogramming how the visuals are updated.

The idea is to avoid flushing the entire board on every move/position update. Instead I calculate the delta between CURRENT_POSITION and new POSITION, and then touch only those squares that need to be touched on every position update.

This solves the flickering problem for Chrome it seems - and the solution works both for immediate updates as well as animated.

Here are some code snippets

  1. let setCurrentPosition return old position
function setCurrentPosition(position) {
    ...
    return oldPos;
}
  1. In widget.position (at the very end) make sure drawPositionInstant is called with the old position
widget.position = function(position, useAnimation) {
  ...
    var old_pos = setCurrentPosition(position);
    drawPositionInstant(old_pos);
  }
};

  1. In doAnimations - make sure drawPositionInstant is called with old position too
// execute an array of animations
function doAnimations(a, oldPos, newPos) {
  ...
  function onFinish() {
    ...
    drawPositionInstant(oldPos);
    ...
}
  1. Add some helper functions for calculating position delta
    /*
      return keys found in obj_a but not in obj_b
     */
    var find_unique = function(obj_a, obj_b) {
    var res = [];
    for (var key in obj_a) {
        if (!obj_a.hasOwnProperty(key)) {continue;}
        if (!obj_b.hasOwnProperty(key)) {
        res.push(key);
        }
    }
    return res;
    };

    /*
      return keys found in both obj_a and obj_b
     */
    var find_common = function (obj_a, obj_b) {
    var res = [];
    for (var key in obj_a) {
        if (!obj_a.hasOwnProperty(key)) {continue;}
        if (obj_b.hasOwnProperty(key)) {
        res.push(key);
        }
    }
    return res;
     };

  1. Update drawPositionInstant to take old position into consideration
function drawPositionInstant(old_pos) {
    var new_pos = CURRENT_POSITION;
    if (old_pos !== undefined) {

    /* optimization - update visuals based on calculating delta between
       old position and new position

       sort squares (position keys) into nonoverlapping sets,
       - removed : squared where the piece need to be removed
       - added : square where a piece need to be added
       - replaced: square where piece must be replaced 

       Note: we actually end up treating added and replaced
       equally, presumably due to a weakness in cleaning up after
       animations. So, in hindsight we would only need two sets
       - removed
       - updated
    */
    var clear_list = find_unique(old_pos, new_pos);
    var add_list = find_unique(new_pos, old_pos);
    var common = find_common(old_pos, new_pos); 
    var replace_list = [];
    for (var i=0; i<common.length; i++) {
        var key = common[i];
        if (old_pos[key] !== new_pos[key]) {
        replace_list.push(key);
        }
    }   

    /* 
       possible optimization 
       make a {} linking all 64 squares to their respective jQuery elements
       so that we don't have to search the DOM on every update
    */

    // clear pieces
    for (var i in clear_list) {
        var square = clear_list[i];
        $('#' + SQUARE_ELS_IDS[square]).find('.' + CSS.piece).remove();
    }
    // replace pieces
    for (var i in replace_list) {
        var square = replace_list[i];
        var $elem = $('#' + SQUARE_ELS_IDS[square]);
        $elem.find('.' + CSS.piece).remove();
        $elem.append(buildPiece(CURRENT_POSITION[square]));
    }
    // add pieces
    for (var i in add_list) {
        var square = add_list[i];
        var $elem = $('#' + SQUARE_ELS_IDS[square]);
        // note - need to remove here, presumably because
        // animation may be going on
        $elem.find('.' + CSS.piece).remove();
        $elem.append(buildPiece(CURRENT_POSITION[square]));
    }
    }
    else {
    // clear the board
    boardEl.find('.' + CSS.piece).remove();
    // add the pieces
    for (var i in CURRENT_POSITION) {
        if (CURRENT_POSITION.hasOwnProperty(i) !== true) { 
        continue;
        }
        $('#' + SQUARE_ELS_IDS[i]).append(buildPiece(CURRENT_POSITION[i]));
    }
    }
}

I think that's it.

Ingar

@jurepolutnik
Copy link

#55 Resolves the problem on Mac OSX / Chrome.

cacheImage() must be called after ChessBoard.init().

@jurepolutnik
Copy link

In addition to previous answer...
In buildPiece() new tag is created and than appended. This causes browser to make a new request to server. To prevent this, configure Expires meta-tag in header accordingly.

@shaktisd
Copy link

shaktisd commented Mar 9, 2014

Resolved the constant refresh issue by adding ,"Cache-Control" : "max-age=604800" to headers.

@knk9
Copy link

knk9 commented Dec 30, 2017

Future reference for anybody trying to solve this issue using Spring: sping-security is overriding the headers. Add this to your WebMvcConfigurerAdapter:

 @Override
    public void addResourceHandlers(ResourceHandlerRegistry registry) {
        registry.addResourceHandler("/img/**")
                .addResourceLocations("classpath:/static/img/")
                .setCachePeriod(12);
    }

@knaveenchand
Copy link

starting the node server resolved this problem for me.

@codejunkiepro
Copy link

Thanks @shaktisd 👍👍

@frederikme
Copy link

Still having this problem sadly, any updates?

@frederikme
Copy link

Firefox & Chrome were working fine, the issue only occurred for Safari on Mac and IOS.
I managed to solve the issue for myself by changing a setting on the server side using Flask, Python.

app = Flask(__name__)
app.send_file_max_age_default = 300

@sesse
Copy link
Author

sesse commented Feb 25, 2022

In a great fit of karma, I've seemingly joined the Chrome team responsible for this, and discovered that there is now a bug that seems possibly related: https://crbug.com/961600

@rorycochrane
Copy link

@frederikme I'm having the same issue, just for Safari on Mac and iOS. Unfortunately the solution you posted doesn't seem to work for me. Did you make any other changes besides setting send_file_max_age_default to 300?

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

No branches or pull requests