Re: Patch to make progressive loading optional

Jon Colverson wrote:
> 
> John Ellis wrote:
> > An option like this I would consider, but the unresponsive-while-loading side
> > effect is what I would like to avoid.
> ...
> > Please try the attached patch (to a clean GQview 1.5.4). Would something like
> > this be sufficient, or is it still too slow?
> >
> > The patch forces delay_flip for all images, the delay_flip ability was
> > introduced as a full screen option in 1.5.2 - this simply enables it everywhere.
>
> Ahh, I failed to notice this option. Might I suggest a different
> caption? Perhaps, "Wait until images have completely loaded before
> changing (in full screen)"? I'm only really concerned with fullscreen
> mode, so I don't mind that it's only used there.

In the next release it will be named "Smooth image flip" under a "full screen"
option group, as that is the original intent of delay_flip - to visually hide
the incremental drawing of the image from the user.


> So, as you say, it comes down to performance. I did a quick check and
> non-progressive loading was about twice as fast. I was able to flip
> through 40 images in 25 seconds, versus 50 seconds for progressive. That
> was with preloading disabled, and the difference would be less
> noticeable in a real world situation where preloading was enabled and I
> wasn't flipping through as fast as I could. For the images I'm normally
> working with, and the hardware I'm using, the non-progressive loading
> was fast enough that the unresponsiveness wasn't noticeable. I'd say the
> overall user experience was better with non-progressive. I can
> understand the problem, though, because I tried a couple of 6000x6000
> astronomical images, and it was unacceptably slow.

I just had another thought that may speed up the progressive loader, GQview
reads in a file in very small chunks of 512 bytes, if you want to try increasing
this, search for 512 in image-load.c (this number should really be set by a
#define). 16k or 32k size chunks may increase the speed a bit.

There are a few #defines that may also be of interest:
   IMAGE_LOAD_BUFFER_COUNT
     effects the number of iterations reading the 512 byte buffer
     per idle loop
   IMAGE_THROTTLE_FACTOR
     the amount buffer iterations above are increased (multiplied) for
     images over IMAGE_THROTTLE_THRESHOLD bytes


> Perhaps my option could be moved to the advanced tab, with a caution on
> the caption, such as: "Load images progressively (non-progressive is
> faster, but makes the interface unresponsive during image loading)".

Yikes, an option that needs a warning.

Hold down the PageDown key in a folder with many larger images - now try to do
anything else, this is why I am against such an option.


> Oh, also, I should mention that a couple of times I've noticed a bug
> which looked like an infinite loop, with the repeating debug message:
> "error accepting socket: Socket operation on non-socket". I didn't think
> to attach a debugger at the time, but I will if I notice it again. I was
> running my build at the time, so there's a chance it was caused by my code.

The warning is generated in remote.c by a failed accept().


Your proposed patch exposes a bug in image_loader_new() not initializing load_fd
with:

  il->load_fd = -1;

thus when progressive is FALSE image_loader_setup() is never called to
initialize load_fd to a valid descriptor, and eventually when
image_loader_stop() is called to clean up it is closing file descriptor 0 (the
ImageLoader struct is initialized with zeros). This pulls the rug from under
some innocent file descriptor somewhere else in the program.



Greetings,
John

-- 
John Ellis <[email protected]>

http://gqview.sourceforge.net <GQview> | http://gqapplets.sourceforge.net
http://gqmpeg.sourceforge.net <GQmpeg> |                  <panel applets>


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now. 
http://productguide.itmanagersjournal.com/