On Wed, Jul 20, 2011 at 12:10:07PM +0300, Alon Levy wrote:
> On Wed, Jul 20, 2011 at 10:53:49AM +0200, Christophe Fergeau wrote:
> > On Wed, Jul 20, 2011 at 11:19:52AM +0300, Alon Levy wrote:
> > > -static void qxl_worker_update_area(QXLWorker *qxl_worker, uint32_t
> > > surface_id,
> > > +static void red_dispatcher_update_area(RedDispatcher *dispatcher,
> > > uint32_t surface_id,
> > > QXLRect *qxl_area, QXLRect
> > > *qxl_dirty_rects,
> > > uint32_t num_dirty_rects, uint32_t
> > > clear_dirty_region)
> > > {
> > > - RedDispatcher *dispatcher = (RedDispatcher *)qxl_worker;
> > > RedWorkerMessage message = RED_WORKER_MESSAGE_UPDATE;
> > > SpiceRect *dirty_rects = spice_new0(SpiceRect, num_dirty_rects);
> > > SpiceRect *area = spice_new0(SpiceRect, 1);
> > > @@ -241,9 +240,16 @@ static void qxl_worker_update_area(QXLWorker
> > > *qxl_worker, uint32_t surface_id,
> > > free(area);
> > > }
> > >
> > > -static void qxl_worker_add_memslot(QXLWorker *qxl_worker, QXLDevMemSlot
> > > *mem_slot)
> > > +static void qxl_worker_update_area(QXLWorker *qxl_worker, uint32_t
> > > surface_id,
> > > + QXLRect *qxl_area, QXLRect
> > > *qxl_dirty_rects,
> > > + uint32_t num_dirty_rects, uint32_t
> > > clear_dirty_region)
> > > +{
> > > + red_dispatcher_update_area((RedDispatcher*)qxl_worker, surface_id,
> > > qxl_area,
> > > + qxl_dirty_rects, num_dirty_rects,
> > > clear_dirty_region);
> > > +}
> >
> >
> > Aren't red_dispatcher_update_area/qxl_worker_update_area redundant? Can't
> > we just keep the old qxl_worker_update_area and do the cast in there?
>
> red_disptacher_update_area is used twice - once by qxl_worker_update_area
> which must remain
> for backward compatibility (QXLWorker.update_area is set to it), and second
> by spice_qxl_update_area
> which goes directly from the QXLInstance to the RedDispatcher without going
> through QXLWorker.
Given the (RedDispatcher*)qxl_worker cast, in this case a RedDispatcher
will also be a QXLWorker and vice versa, so I don't understand why we need
a wrapper whose only added value is an obvious cast.
> > Will the function pointer above go away? My understanding is that the
> > functions added below are meant to replace these?
>
> They can't go away as long as we want to maintain backwards compatibility.
Ok, I'd have gone with using these to implement spice_qxl_wakeup et al:
void spice_qxl_wakeup(QXLInstance *instance)
{
QXLWorker *worker = (QXLWorker *)instance->st->dispatcher;
ASSERT(worker->wakeup != NULL)
worker->wakeup(worker);
}
or something like that
But maybe that's something kraxel wanted to avoid.
Christophe
pgpWDYFGeTI83.pgp
Description: PGP signature
_______________________________________________ Spice-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/spice-devel
