Gergő Tisza has uploaded a new change for review. https://gerrit.wikimedia.org/r/129564
Change subject: [WIP] Make progress bar less erratic ...................................................................... [WIP] Make progress bar less erratic Seems to work (apart from the tests), but I don't really like it, I'll refactor a bit more. We should at least get rid of all the manual "if this is not the current image, exit" stuff in the callbacks, it is missing at some places and an easy source of errors. Change-Id: I2557abcec173691ffce21185bf1a939f1644ba8c Mingle: https://wikimedia.mingle.thoughtworks.com/projects/multimedia/cards/489 --- M resources/mmv/mmv.js M resources/mmv/ui/mmv.ui.canvas.js 2 files changed, 87 insertions(+), 50 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/MultimediaViewer refs/changes/64/129564/1 diff --git a/resources/mmv/mmv.js b/resources/mmv/mmv.js index ed64043..64c2bfa 100755 --- a/resources/mmv/mmv.js +++ b/resources/mmv/mmv.js @@ -239,9 +239,7 @@ metadataPromise, start, viewer = this, - $initialImage = $( initialImage ), - fileWidth = image.originalWidth, - fileHeight = image.originalHeight; + $initialImage = $( initialImage ); this.currentIndex = image.index; @@ -273,48 +271,13 @@ start = $.now(); - // Reset the progress bar, it could be at any state if we're calling loadImage - // while another image is already loading - viewer.ui.panel.progressBar.percent( 0 ); - imagePromise = this.fetchThumbnailForLightboxImage( image, imageWidths.real ); - // Check that the image hasn't already been loaded - if ( imagePromise.state() === 'pending' ) { - // Animate it to 5 to give a sense to something is happening, even if we're stuck - // waiting for server-side processing, such as thumbnail (re)generation - viewer.ui.panel.progressBar.percent( 5 ); - } + viewer.displayPlaceholderThumbnail( image, $initialImage, imageWidths ); - if ( fileWidth > 0 && fileHeight > 0 ) { - viewer.displayPlaceholderThumbnail( { width : fileWidth , height : fileHeight }, - $initialImage, - imageWidths ); - } else { - this.imageInfoProvider.get( image.filePageTitle ).done( function ( imageInfo ) { - if ( viewer.currentIndex !== image.index ) { - return; - } + this.setupProgressBar( image, imagePromise ); - viewer.displayPlaceholderThumbnail( imageInfo, $initialImage, imageWidths ); - } ); - } - - imagePromise.progress( function ( thumbnailInfoResponse, imageResponse ) { - if ( viewer.currentIndex !== image.index ) { - return; - } - - if ( viewer.ui - && viewer.ui.panel - && imageResponse.length === 2 - && imageResponse[ 1 ] > 5 ) { - viewer.ui.panel.progressBar.percent( imageResponse[ 1 ] ); - } - } ).done( function ( thumbnail, imageElement ) { - // Fallback in case the browser doesn't have fancy progress updates - viewer.ui.panel.progressBar.percent( 100 ); - + imagePromise.done( function ( thumbnail, imageElement ) { if ( viewer.currentIndex !== image.index ) { return; } @@ -406,19 +369,92 @@ /** * Display the blurred thumbnail from the page - * @param {mw.mmv.model.Image} imageInfo + * @param {mw.mmv.LightboxImage} image * @param {jQuery} $initialImage The thumbnail from the page * @param {mw.mmv.model.ThumbnailWidth} imageWidths */ - MMVP.displayPlaceholderThumbnail = function ( imageInfo, $initialImage, imageWidths ) { + MMVP.displayPlaceholderThumbnail = function ( image, $initialImage, imageWidths ) { + var viewer = this, + size = { width : image.originalWidth, height : image.originalHeight }; + // If the actual image has already been displayed, there's no point showing the blurry one if ( this.realThumbnailShown ) { return; } - this.blurredThumbnailShown = this.ui.canvas.maybeDisplayPlaceholder( - imageInfo, $initialImage, imageWidths ); + if ( !size.width || !size.height ) { + // These values are added to the HTML by MediaViewer via a PHP hook, and can be missing + // in exotic circumstances, e.g. when the extension has only been enabled recently + // and the HTML cache has not cleared yet. If that is the case, we need to fetch the + // size from the API first. + this.imageInfoProvider.get( image.filePageTitle ).done( function ( imageInfo ) { + // Make sure the user has not navigated away while we were waiting for the size + if ( viewer.currentIndex === image.index ) { + image.originalWidth = imageInfo.width; + image.originalHeight = imageInfo.height; + viewer.displayPlaceholderThumbnail( image, $initialImage, imageWidths ); + } + } ); + } else { + this.blurredThumbnailShown = this.ui.canvas.maybeDisplayPlaceholder( + size, $initialImage, imageWidths ); + } }; + + /** + * Displays a progress bar for the image loading, if necessary, and sets up handling of + * all the related callbacks. + * FIXME would be nice to pass a simple promise which only returns a single number + * and does not fire when the image is not visible + * @param {mw.mmv.LightboxImage} image + * @param {jQuery.Promise.<mw.mmv.model.Thumbnail, HTMLImageElement>} imagePromise + */ + MMVP.setupProgressBar = function ( image, imagePromise ) { + var viewer = this; + + // Reset the progress bar, it could be at any state if we're calling loadImage + // while another image is already loading + // FIXME we should probably jump to the current progress instead + viewer.ui.panel.progressBar.percent( 0 ); + + if ( imagePromise.state() !== 'pending' ) { + // image has already loaded (or failed to load) - nothing to do + return; + } + + // FIXME this is all wrong, we might be navigating back to a half-loaded image + + // Animate progress bar to 5 to give a sense to something is happening, even if we're + // stuck waiting for server-side processing, such as thumbnail (re)generation + viewer.ui.panel.progressBar.percent( 5 ); + + imagePromise.progress( function ( thumbnailInfoResponse, imageResponse ) { + // FIXME this should be explained in a comment + var progress = imageResponse[1]; + + if ( viewer.currentIndex !== image.index ) { + return; + } + + // We started from 5, don't move backwards + if ( progress > 5 ) { + viewer.ui.panel.progressBar.percent( progress ); + } + } ).done( function () { + if ( viewer.currentIndex !== image.index ) { + return; + } + + // Fallback in case the browser doesn't have fancy progress updates + viewer.ui.panel.progressBar.percent( 100 ); + } ).fail( function () { + if ( viewer.currentIndex !== image.index ) { + return; + } + // Hide progress bar on error + viewer.ui.panel.progressBar.percent( 0 ); + } ); + } /** * Preload this many prev/next images to speed up navigation. @@ -592,6 +628,7 @@ * Loads size-dependent components of a lightbox - the thumbnail model and the image itself. * @param {mw.mmv.LightboxImage} image * @param {number} width the width of the requested thumbnail + * @returns {jQuery.Promise.<mw.mmv.model.Thumbnail, HTMLImageElement>} */ MMVP.fetchThumbnailForLightboxImage = function ( image, width ) { return this.fetchThumbnail( diff --git a/resources/mmv/ui/mmv.ui.canvas.js b/resources/mmv/ui/mmv.ui.canvas.js index e08f5ec..c691b8d 100644 --- a/resources/mmv/ui/mmv.ui.canvas.js +++ b/resources/mmv/ui/mmv.ui.canvas.js @@ -177,23 +177,23 @@ * We set SVG files to the maximum screen size available. * Assumes set function called before. * - * @param {mw.mmv.model.Image} imageInfo + * @param {{width: number, height: number}} size * @param {jQuery} $imagePlaceholder Image placeholder to be displayed while the real image loads. * @param {mw.mmv.model.ThumbnailWidth} imageWidths * @returns {boolean} Whether the image was blured or not */ - C.maybeDisplayPlaceholder = function ( imageInfo, $imagePlaceholder, imageWidths ) { + C.maybeDisplayPlaceholder = function ( size, $imagePlaceholder, imageWidths ) { var targetWidth, targetHeight, blowupFactor, blurredThumbnailShown = false; // Assume natural thumbnail size¸ - targetWidth = imageInfo.width; - targetHeight = imageInfo.height; + targetWidth = size.width; + targetHeight = size.height; // If the image is bigger than the screen we need to resize it - if ( imageInfo.width > imageWidths.cssWidth ) { // This assumes imageInfo.width in CSS units + if ( size.width > imageWidths.cssWidth ) { // This assumes imageInfo.width in CSS units targetWidth = imageWidths.cssWidth; targetHeight = imageWidths.cssHeight; } -- To view, visit https://gerrit.wikimedia.org/r/129564 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I2557abcec173691ffce21185bf1a939f1644ba8c Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/MultimediaViewer Gerrit-Branch: master Gerrit-Owner: Gergő Tisza <[email protected]> _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
