Well, I don't pretend to know anything about X, but this is the commit that added the code in question: commit 297fd0d0755bda698be1d0b30cc60a41d7673c0b Author: Dave Airlie <airlied at redhat.com> Date: Tue Oct 16 16:15:16 2012 +1000 nouveau/dri2: fix pixmap/window offset calcs. This should fix prime rendering under kwin, and not break it under the others. Signed-off-by: Dave Airlie <airlied at redhat.com> diff --git a/src/nouveau_dri2.c b/src/nouveau_dri2.c index 71cff26..7bd0b3a 100644 --- a/src/nouveau_dri2.c +++ b/src/nouveau_dri2.c @@ -165,9 +165,9 @@ nouveau_dri2_copy_region2(ScreenPtr pScreen, DrawablePtr pDraw, RegionPtr pRegio translate = TRUE; if (translate && pDraw->type == DRAWABLE_WINDOW) { - WindowPtr pWin = (WindowPtr)pDraw; - off_x = pWin->origin.x; - off_y = pWin->origin.y; + PixmapPtr pPix = get_drawable_pixmap(pDraw); + off_x = pDraw->x - pPix->screen_x; + off_y = pDraw->y - pPix->screen_y; } pGC = GetScratchGC(pDraw->depth, pScreen); Now I sort of assume that pDraw->x == pWin->origin.x. But... who knows. -ilia On Tue, Jul 14, 2015 at 5:46 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:> On 14 July 2015 at 22:17, Ilia Mirkin <imirkin at alum.mit.edu> wrote: >> --- >> src/nouveau_dri2.c | 15 ++++++++++----- >> 1 file changed, 10 insertions(+), 5 deletions(-) >> >> diff --git a/src/nouveau_dri2.c b/src/nouveau_dri2.c >> index f22e319..4398559 100644 >> --- a/src/nouveau_dri2.c >> +++ b/src/nouveau_dri2.c >> @@ -142,6 +142,7 @@ nouveau_dri2_copy_region2(ScreenPtr pScreen, DrawablePtr pDraw, RegionPtr pRegio >> NVPtr pNv = NVPTR(xf86ScreenToScrn(pScreen)); >> RegionPtr pCopyClip; >> GCPtr pGC; >> + PixmapPtr pPix; >> DrawablePtr src_draw, dst_draw; >> Bool translate = FALSE; >> int off_x = 0, off_y = 0; >> @@ -170,9 +171,13 @@ nouveau_dri2_copy_region2(ScreenPtr pScreen, DrawablePtr pDraw, RegionPtr pRegio >> } >> >> if (translate && pDraw->type == DRAWABLE_WINDOW) { >> - PixmapPtr pPix = get_drawable_pixmap(pDraw); >> - off_x = pDraw->x - pPix->screen_x; >> - off_y = pDraw->y - pPix->screen_y; >> + off_x = pDraw->x; >> + off_y = pDraw->y; >> +#ifdef COMPOSITE >> + pPix = get_drawable_pixmap(pDraw); >> + off_x -= pPix->screen_x; >> + off_y -= pPix->screen_y; >> +#endif > If I understand things correctly both ati and the intel ddx seems set > the offsets to zero when composite is missing. > I doubt that many people build xserver without it though :-) > > -Emil
Upon closer look it seem that I was a bit off - the ati/intel ddx does not set off_x/y to zero when built without composite*. ... in copy_region2, they do so when working with video - XvAdaptorPtr::ddPutImage/XF86VideoAdaptorPtr::PutImage. Does this mean that they need a similar update ? What are the symptoms of the issue ? Thanks Emil On 14 July 2015 at 23:51, Ilia Mirkin <imirkin at alum.mit.edu> wrote:> Well, I don't pretend to know anything about X, but this is the commit > that added the code in question: > > commit 297fd0d0755bda698be1d0b30cc60a41d7673c0b > Author: Dave Airlie <airlied at redhat.com> > Date: Tue Oct 16 16:15:16 2012 +1000 > > nouveau/dri2: fix pixmap/window offset calcs. > > This should fix prime rendering under kwin, and not break it under the > others. > > Signed-off-by: Dave Airlie <airlied at redhat.com> > > diff --git a/src/nouveau_dri2.c b/src/nouveau_dri2.c > index 71cff26..7bd0b3a 100644 > --- a/src/nouveau_dri2.c > +++ b/src/nouveau_dri2.c > @@ -165,9 +165,9 @@ nouveau_dri2_copy_region2(ScreenPtr pScreen, > DrawablePtr pDraw, RegionPtr pRegio > translate = TRUE; > > if (translate && pDraw->type == DRAWABLE_WINDOW) { > - WindowPtr pWin = (WindowPtr)pDraw; > - off_x = pWin->origin.x; > - off_y = pWin->origin.y; > + PixmapPtr pPix = get_drawable_pixmap(pDraw); > + off_x = pDraw->x - pPix->screen_x; > + off_y = pDraw->y - pPix->screen_y; > } > > pGC = GetScratchGC(pDraw->depth, pScreen); > > > Now I sort of assume that pDraw->x == pWin->origin.x. But... who knows. > > -ilia > > > On Tue, Jul 14, 2015 at 5:46 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote: >> On 14 July 2015 at 22:17, Ilia Mirkin <imirkin at alum.mit.edu> wrote: >>> --- >>> src/nouveau_dri2.c | 15 ++++++++++----- >>> 1 file changed, 10 insertions(+), 5 deletions(-) >>> >>> diff --git a/src/nouveau_dri2.c b/src/nouveau_dri2.c >>> index f22e319..4398559 100644 >>> --- a/src/nouveau_dri2.c >>> +++ b/src/nouveau_dri2.c >>> @@ -142,6 +142,7 @@ nouveau_dri2_copy_region2(ScreenPtr pScreen, DrawablePtr pDraw, RegionPtr pRegio >>> NVPtr pNv = NVPTR(xf86ScreenToScrn(pScreen)); >>> RegionPtr pCopyClip; >>> GCPtr pGC; >>> + PixmapPtr pPix; >>> DrawablePtr src_draw, dst_draw; >>> Bool translate = FALSE; >>> int off_x = 0, off_y = 0; >>> @@ -170,9 +171,13 @@ nouveau_dri2_copy_region2(ScreenPtr pScreen, DrawablePtr pDraw, RegionPtr pRegio >>> } >>> >>> if (translate && pDraw->type == DRAWABLE_WINDOW) { >>> - PixmapPtr pPix = get_drawable_pixmap(pDraw); >>> - off_x = pDraw->x - pPix->screen_x; >>> - off_y = pDraw->y - pPix->screen_y; >>> + off_x = pDraw->x; >>> + off_y = pDraw->y; >>> +#ifdef COMPOSITE >>> + pPix = get_drawable_pixmap(pDraw); >>> + off_x -= pPix->screen_x; >>> + off_y -= pPix->screen_y; >>> +#endif >> If I understand things correctly both ati and the intel ddx seems set >> the offsets to zero when composite is missing. >> I doubt that many people build xserver without it though :-) >> >> -Emil
On 15 July 2015 at 14:12, Emil Velikov <emil.l.velikov at gmail.com> wrote:> Upon closer look it seem that I was a bit off - the ati/intel ddx does > not set off_x/y to zero when built without composite*. > > ... in copy_region2, they do so when working with video - > XvAdaptorPtr::ddPutImage/XF86VideoAdaptorPtr::PutImage. > > Does this mean that they need a similar update ? What are the symptoms > of the issue ? >Can anyone give some rough pointers to the appropriate reading material ? As mentioned neither ati nor the intel ddx follow this approach so I'm a bit puzzled. From a quick test there is no "build fail" if when COMPOSITE is missing. Am I missing something obvious/trivial ? Thanks Emil