Re: [g-a-devel][Fwd: GNOME magnifier changes]



Hi Brian:

Thanks a lot for doing this work and finding areas for improvement. 
Generally the patch looks good, but there are a couple of things I am
concerned about.
,
Firstly, I'd like to see this as two patches, one for the timing-test
data only, and a subsequent patch that changes behavior and priorities
of idle handlers, etc.  That is so that we can actually benefit
immediately from the benchmark information and do before/after
comparisons on the algorithm changes you propose.


Also :
> In zoom_region_queue_update, I now add the zoom_region_process_updates
> idle handler with a priority of "GDK_PRIORITY_REDRAW - 1".  This
> ensures that the scaling is done before GTK updates the screen, which
> (if my understanding is correct) will ensure that the update refreshes
> the screen with the most current data, rather than from the data from
> the last iteration of the loop.  This seems, to my eyeball, to make the
> magnifier look 

My initial reaction is that this is not correct.  Process_updates should
never block a gtk update; the objective is to make panning response
maximally responsive, and to update the screen later.  

The other performance-related changes look good.

I would prefer to see some summary output in the --timing-output
results; could you give min/max/average values for things like scrolling
time, frames-per-second in scrolling mode, and pixels-moved-per-second ?

That said, this work is enormously helpful and I can hardly wait to get
the timing test information into cvs.  I also am very much looking
forward to benchmarking the individual performance changes; however I
would prefer that the initial patches do not change the process_updates
versus gdk_expose processing order.

best regards,

Bill




[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]