Re: Abort glyph rendering if everything is clipped

> But then you are doing extra work for the "normal" case, and you are
> also making the code less readable; not a good trade-off IMO.

Well --- trading a memory allocation for an extra comparision instruction makes
it a bit trickier to decide, but I take your point.
> You
> could always move the whole clipping rectangle stuff to avoid the
> HeapAlloc (which would also fix a memory leak in the existing code...)
> if you prefer.

Unfortunately the clipping rectangle calculation is using the glyphs array that
is allocated by the HeapAlloc.  What I have right now is the clipping rectangle
stuff after the HeapAlloc and GetGlyphIndicies call, and I've added code to free
the allocated memory if we bail out...  Here's what it looks like (my mailer
will wrap lines at 72 characters --- If anyone knows how to disable that in
NotesR5, *please* let me know!):  (this is around line 940 in xrender.c)

if(flags & ETO_GLYPH_INDEX)
        glyphs = (LPWORD)wstr;
    else {
        glyphs = HeapAlloc(GetProcessHeap(), 0, count * sizeof(WCHAR));
        GetGlyphIndicesW(hdc, wstr, count, glyphs, 0);
    }

if(flags & (ETO_CLIPPED | ETO_OPAQUE)) {
        if(!lprect) {
         if(flags & ETO_CLIPPED) {
          if(glyphs != wstr) HeapFree(GetProcessHeap(), 0, glyphs);
          return FALSE;
         }
         GetTextExtentPointI(hdc, glyphs, count, &sz);
         done_extents = TRUE;
         rc.left = x;
         rc.top = y;
         rc.right = x + sz.cx;
         rc.bottom = y + sz.cy;
     } else {
         if (IsRectEmpty(lprect)) {
          if(glyphs != wstr) HeapFree(GetProcessHeap(), 0, glyphs);
          return TRUE;
         }
         rc = *lprect;
     }

LPtoDP(physDev->hdc, (POINT*)&rc, 2);

if(rc.left > rc.right) {INT tmp = rc.left; rc.left = rc.right; rc.right =
tmp;}
     if(rc.top > rc.bottom) {INT tmp = rc.top; rc.top = rc.bottom; rc.bottom =
tmp;}
    }

If this looks good, I can submit a new patch this afternoon...
> But I think keeping that code readable is more
> important than wringing every last nanosecond of the empty clip
> rectangle case.

I agree --- But I'd rather do both if we can.

Warren

Abort glyph rendering if everything is clipped

[Alexandre Julliard]

[e-mail] writes:
> if(flags & (ETO_CLIPPED | ETO_OPAQUE)) {
>     if(!lprect) {
>      if(flags & ETO_CLIPPED) {
>       if(glyphs != wstr) HeapFree(GetProcessHeap(), 0, glyphs);
>       return FALSE;
>      }
>      GetTextExtentPointI(hdc, glyphs, count, &sz);
>      done_extents = TRUE;
>      rc.left = x;
>      rc.top = y;
>      rc.right = x + sz.cx;
>      rc.bottom = y + sz.cy;
>  } else {
>      if (IsRectEmpty(lprect)) {
>       if(glyphs != wstr) HeapFree(GetProcessHeap(), 0, glyphs);
>       return TRUE;
>      }
>      rc = *lprect;
>  }
>
>  LPtoDP(physDev->hdc, (POINT*)&rc, 2);
>
>  if(rc.left > rc.right) {INT tmp = rc.left; rc.left = rc.right; rc.right =
>
> tmp;}
>      if(rc.top > rc.bottom) {INT tmp = rc.top; rc.top = rc.bottom; rc.bottom =
> tmp;}
>     }
>
> If this looks good, I can submit a new patch this afternoon...

That looks better yes; it's still a bit complex IMO, but I'm not sure
it's possible to do much better with that glyph index thing. Still I'm
wondering if you shouldn't do the IsRectEmpty check after the LPtoDP
and coordinates swap.