[haiku-appserver] Re: update code.

Hi Adi,

> > Ok, you're absolutely right about that. But notice how you cannot 
> > tell 
> > the Accelerant about any clipping region? That's because the 
> > app_server 
> > has made sure that the rects to be moved actually make sense. In 
> > other 
> > words it's the rects that remain *after* clipping has already been 
> > applied.
> 
>       Aha, now I get what you mean.
> 
> > Now, there is no problem to apply the clipping inside the 
> > DisplayDriver implementation. BUT app_server needs to make sure 
> > that 
> > the view  requesting the CopyBits() gets invalidated whereever 
> > there
> > were no contents on screen before the blit.
> 
>       I'm not sure I catch this one.

Read in BeBook BView::CopyBits().

MyView::ScrollBy(float x, float y)
{
        BRect bounds = (Bounds());
        CopyBits(bounds, bounds.OffsetBySelf(x, y));
}

Two things happen here. The BViews contents are shifted by x and y. BUT 
what happens with the area, that was previously no within Bounds()? For 
this area, the app_server triggers an invalidation and therefor Draw() 
to be called (all automatically). What I'm saying in my above paragraph 
is, that DisplayDriver cannot do this job for you. It *can* take care 
of the clipping. But I think how you're using DisplayDriver is you 
directly call it with the parameters you get from the client, and that 
is something you cannot do anyways. I'm further saying that the 
accelerant hook does not do the clipping, but it can however be done in 
DisplayDriver before passing it on to the accelerant.


> > That is something the 
> > DisplayDriver cannot do for app_server.
> > Same goes for the other function with blitting all those regions. 
> > If it 
> > can be assumed by the DisplayDriver, that the regions are 
> > preprocessed, 
> > so that they a) don't overlap (so that the same area doesn't get 
> > moved 
> > twice or more), and b) that the destination rects don't overlap for 
> > the 
> > different offsets that they may have, this function could be 
> > accelerated 3 times. If those requirements cannot be guaranteed, 
> > then 
> > indeed, your implementation is the fail save one. However, it would 
> > draw areas on screen, which would have to be redrawn by the views 
> > anyhow. Maybe I find some time to illustrate the problem. I 
> > understand 
> > perfectly well what you guys had in mind when designing this 
> > function, 
> > however, I'm saying it is not going to visually work without some 
> > additional precautions.
> 
>       :-) I did not understood much from the above paragraph, sorry.
> 
>       What I did understood is that you complain this method CCR()
> should not be inside DD because of clipping operations(?)

No, read above. It can perform the clipping, but you need to be aware 
of the BView regions that would need to be ivalidated inside app_server 
anyways. So what you _cannot_ do is to just call the DisplayDriver 
method with the parameters comming from the client, even if 
DisplayDriver would perform the clipping for you. That's because then 
you don't know which regions you need to trigger invalidation for.

> and there are
> performance issues if overlapping occurs.

I hope it is clear now which kind of overlapping I mean. It would be 
much easier if I draw this out on paper, I guess... :-))

>       I think this function must stay in DD as the HW accelerated 
> subclass
> should pass the rectangles found in those regions to the accelerant, 
> by
> way of screen_to_screen_blit() hook function. (That HW accelerated 
> subclass
> should build a list of blit_params and pass to the accelerant) From 
> this point
> it's accelerant's job how to handle things. And he can do _a lot_ 
> better than
> what I did in CCR(). He has enough video memory at its disposal so 
> that it can
> copy really fast from on-screen to a part of free video memory and 
> then back
> again. And if I'm not wrong this is fast enough to be done during a 
> retrace.

I'm pretty sure it does an in-place copy. It just figures out in which 
direction to read and write the bytes, so that it doesn't read from 
locations that have already been written to during that operation. 
Suppose this:

12345678xxxxxxx

This is a row of pixels in memory. The numbers stand for colors. You 
want to copy the pixels 4 coordinates to the right, this is what you 
want:

123412345678xxx

Suppose you do this in-place like this:

int32 xOffset = 4;

uint32* src = buffer;
uint32* dst = src + xOffset;

for (uint32 x = 0; x < width; x++) {
        *dst = *src;
        dst++;
        src++;
}

What you get is this (in each iteration):

12341678xxxxxxx
12341278xxxxxxx
12341238xxxxxxx
12341234xxxxxxx
123412341xxxxxx
1234123412xxxxx
12341234123xxxx
123412341234xxx

Ups? Where did 5678 go? You have overwritten them in the first for 
iterations. That's why you need to do it smarter:

// xOffset is 4

int32 xIncrement;
if (xOffset > 0) {
        // copy right to left
        src* += width;
        xIncrement = -1;
} else {
        // copy left to right
        xIncrement = 1;
}

dst = src + xOffset;

for (uint32 x = 0; x < width; x++) {
        *dst = *src;
        dst += xIncrement;
        src += xIncrement;
}

And here is the result of each iteration:

12345678xxx8xxx
12345678xx78xxx
12345678x678xxx
123456785678xxx
123456745678xxx
123456345678xxx
123452345678xxx
123412345678xxx

And that's correct. Ok, I hope you understand what I mean by in-place 
copy and what I mean by overlapping. That is the one kind of 
overlapping. In this case, the destination area overlaps the source 
area in memory, but that is _no_ problem when you take that into 
account in your copy algorithm. And I think this is what the graphics 
engine in the accelerant is doing as well. It does not copy to out of 
screen regions and then back into visible region at the offset. Unless 
Rudolf convinces me otherwise... :-)

>       We kinda' need CCR() although it doesn't fully comply with DD
> specifications. :-)

I'm not saying it should go away, I'm just saying that certain criteria 
must be met for it to be implemented efficiently. Now, here is one 
criterium: The regions that it gets passed should not overlap. And now 
I'm talking about a different kind of overlapping. Let's suppose you 
have a function:

void DisplayDriver::_MoveRect(BRect rect, BPoint offset);

This function handles all the magic of doing in-place moving of the 
rect and it handles also the case where the rect at the offset overlaps 
the original rect. I have shown above that this is possible.

What I'm saying now, is that in the cause of one CCR() call, as you 
call it, this function (MoveRect()) should never have to be called for 
the same source rect twice or more. Or else the graphics would be 
completely screwed if you do in-place copying. Ensuring this by 
app_server prior to calling CCR() is just much smarter. And I'm telling 
you, you need this logic in app_server anyways, or else you don't know 
which regions of BViews need to be invalidated, because of the resizing 
or scrolling (for which this CCR() thing is meant to be in the first 
place).

Uff. That's a long mail. Sorry, but I hope all is clear now.

Best regards,
-Stephan

Other related posts: