Skip to content

Commit

Permalink
Get rid of RevRGB & co (#402)
Browse files Browse the repository at this point in the history 
It turns out it's not just LVImg that wants inverted alpha: it's *all* CRe...
So, we'll be handling the pixel format conversion entirely in KOReader instead, as dealing with the alpha here is kind of hairy without basically rewiring all the things.

:(
  • Loading branch information
@NiLuJe
NiLuJe authored Dec 23, 2020
1 parent 600811f commit 8254cef
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 40 deletions.
19 changes: 0 additions & 19 deletions crengine/include/lvdrawbuf.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -404,29 +404,10 @@ class LVGrayDrawBuf : public LVBaseDrawBuf
virtual void DrawLine(int x0, int y0, int x1, int y1, lUInt32 color0, int length1, int length2, int direction=0);
};

// NOTE: By default, CRe assumes RGB (array order) actually means BGR
// We don't, so, instead of fixing this at the root (i.e., in a *lot* of places),
// we simply swap R<->B when rendering to 32bpp, limiting the tweaks to lvdrawbuf
// c.f., https://github.com/koreader/koreader-base/pull/878#issuecomment-476723747
#ifdef CR_RENDER_32BPP_RGB_PXFMT
inline lUInt32 RevRGB( lUInt32 cl ) {
return ((cl<<16)&0xFF0000) | ((cl>>16)&0x0000FF) | (cl&0x00FF00);
}

inline lUInt32 RevRGBA( lUInt32 cl ) {
// Swap B <-> R, keep G & A
return ((cl<<16)&0x00FF0000) | ((cl>>16)&0x000000FF) | (cl&0xFF00FF00);
}
#else
inline lUInt32 RevRGB( lUInt32 cl ) {
return cl;
}

inline lUInt32 RevRGBA( lUInt32 cl ) {
return cl;
}
#endif

inline lUInt32 rgb565to888( lUInt32 cl ) {
return ((cl & 0xF800)<<8) | ((cl & 0x07E0)<<5) | ((cl & 0x001F)<<3);
}
Expand Down
4 changes: 1 addition & 3 deletions crengine/src/lvdocview.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1114,9 +1114,7 @@ void LVDocView::drawCoverTo(LVDrawBuf * drawBuf, lvRect & rc) {
if (dst_dy > rc.height() * 6 / 8)
dst_dy = imgrc.height();
//CRLog::trace("drawCoverTo() - drawing image");
// It's best to use a 16bpp LVColorDrawBuf as the intermediate buffer,
// as using 32bpp would mess colors up when drawBuf is itself 32bpp.
LVColorDrawBuf buf2(src_dx, src_dy, 16);
LVColorDrawBuf buf2(src_dx, src_dy, 32);
buf2.Draw(imgsrc, 0, 0, src_dx, src_dy, true);
drawBuf->DrawRescaled(&buf2, imgrc.left + (imgrc.width() - dst_dx) / 2,
imgrc.top + (imgrc.height() - dst_dy) / 2, dst_dx, dst_dy, 0);
Expand Down
30 changes: 16 additions & 14 deletions crengine/src/lvdrawbuf.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -587,14 +587,14 @@ class LVImageScaledDrawCallback : public LVImageDecoderCallback
}
} else if ( alpha == 0 ) {
// Fully opaque, plot it as-is
row[ x ] = RevRGB(cl) ^ rgba_invert;
row[ x ] = cl ^ rgba_invert;
} else {
if ((row[x] & 0xFF000000) == 0xFF000000) {
// Plot it as-is if *buffer* pixel is transparent
row[ x ] = RevRGBA(cl) ^ rgba_invert;
row[ x ] = cl ^ rgba_invert;
} else {
// NOTE: This *also* has a "fully opaque" shortcut... :/
ApplyAlphaRGB( row[x], RevRGB(cl), alpha );
ApplyAlphaRGB( row[x], cl, alpha );
// Invert post-blending to avoid potential stupidity...
row[ x ] ^= rgba_invert;
}
Expand Down Expand Up @@ -1495,7 +1495,7 @@ void LVColorDrawBuf::Clear( lUInt32 color )
}
}
} else {
const lUInt32 cl32 = RevRGBA(color);
const lUInt32 cl32 = color;
for (int y=0; y<_dy; y++)
{
lUInt32 * __restrict dst = (lUInt32 *)GetScanLine(y);
Expand Down Expand Up @@ -1680,9 +1680,9 @@ void LVColorDrawBuf::FillRect( int x0, int y0, int x1, int y1, lUInt32 color )
for (int x=x0; x<x1; x++)
{
if (alpha)
ApplyAlphaRGB(line[x], RevRGB(color), alpha);
ApplyAlphaRGB(line[x], color, alpha);
else
line[x] = RevRGBA(color);
line[x] = color;
}
}
}
Expand Down Expand Up @@ -1712,7 +1712,7 @@ void LVColorDrawBuf::DrawLine(int x0, int y0, int x1, int y1, lUInt32 color0, in
}
}
} else {
const lUInt32 cl32 = RevRGBA(color0);
const lUInt32 cl32 = color0;
for (int y=y0; y<y1; y++)
{
lUInt32 * __restrict line = (lUInt32 *)GetScanLine(y);
Expand Down Expand Up @@ -1758,7 +1758,7 @@ void LVColorDrawBuf::FillRectPattern( int x0, int y0, int x1, int y1, lUInt32 co
for (int x=x0; x<x1; x++)
{
const lUInt8 patternBit = (patternMask << (x&7)) & 0x80;
line[x] = patternBit ? RevRGBA(color1) : RevRGBA(color0);
line[x] = patternBit ? color1 : color0;
}
}
}
Expand Down Expand Up @@ -1936,7 +1936,7 @@ void LVColorDrawBuf::Draw( int x, int y, const lUInt8 * bitmap, int width, int h
bitmap += bmp_width;
}
} else {
const lUInt32 bmpcl32 = RevRGBA(bmpcl);
const lUInt32 bmpcl32 = bmpcl;

while (height--)
{
Expand All @@ -1956,7 +1956,9 @@ void LVColorDrawBuf::Draw( int x, int y, const lUInt8 * bitmap, int width, int h
const lUInt8 alpha = 0x7F-opaque;
const lUInt32 cl1 = ((alpha*((*dst)&0xFF00FF) + opaque*(bmpcl32&0xFF00FF))>>7) & 0xFF00FF;
const lUInt32 cl2 = ((alpha*((*dst)&0x00FF00) + opaque*(bmpcl32&0x00FF00))>>7) & 0x00FF00;
// NOTE: We're never restoring an alpha byte here... Is it supposed to be 0x00?
// NOTE: We're skipping the alpha byte here, because CRe uses inverted alpha :(
// Which means that the masking shenanigans above ensure it's 0x00, fully opaque...
// (c.f., ApplyAlphaRGB, which does the same thing without the weird half-precision hack).
*dst = cl1 | cl2;
}
dst++;
Expand Down Expand Up @@ -2616,7 +2618,7 @@ void LVColorDrawBuf::DrawTo( LVDrawBuf * __restrict buf, int x, int y, int optio
lUInt32 * __restrict dst = ((lUInt32 *)buf->GetScanLine(y + yy)) + x;
for (int xx = 0; xx < _dx; xx++) {
if (x+xx >= clip.left && x + xx < clip.right) {
*dst = RevRGBA(*src);
*dst = *src;
}
dst++;
src++;
Expand Down Expand Up @@ -2761,7 +2763,7 @@ void LVColorDrawBuf::DrawOnTop( LVDrawBuf * __restrict buf, int x, int y)
lUInt32 * __restrict dst = ((lUInt32 *)buf->GetScanLine(y + yy)) + x;
for (int xx = 0; xx < _dx; xx++) {
if (x+xx >= clip.left && x + xx < clip.right) {
if(*src!=0) *dst = RevRGBA(*src);
if(*src!=0) *dst = *src;
}
dst++;
src++;
Expand Down Expand Up @@ -2932,7 +2934,7 @@ void LVColorDrawBuf::DrawRescaled(const LVDrawBuf * __restrict src, int x, int y
dst[x + xx] = rgb888to565(cl);
} else {
lUInt32 * __restrict dst = (lUInt32 *)GetScanLine(y + yy);
dst[x + xx] = RevRGBA(cl);
dst[x + xx] = cl;
}
}
}
Expand All @@ -2951,7 +2953,7 @@ void LVColorDrawBuf::DrawRescaled(const LVDrawBuf * __restrict src, int x, int y
dst[x + xx] = rgb888to565(cl);
} else {
lUInt32 * __restrict dst = (lUInt32 *)GetScanLine(y + yy);
dst[x + xx] = RevRGBA(cl);
dst[x + xx] = cl;
}
}
}
Expand Down
7 changes: 3 additions & 4 deletions crengine/src/lvrend.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8464,10 +8464,9 @@ void DrawBackgroundImage(ldomNode *enode,LVDrawBuf & drawbuf,int x0,int y0,int d
// Ready to have crengine do all the work.
/* Looks like we don't need that:

// (Inspired from LVDocView::drawPageBackground(), we have to do it that complex
// way to avoid memory leaks; and we have to use a 16bpp LVColorDrawBuf,
// 32bpp would mess colors up).
LVRef<LVColorDrawBuf> buf = LVRef<LVColorDrawBuf>( new LVColorDrawBuf(img_w, img_h, 16) );
// (Inspired from LVDocView::drawPageBackground(),
// we have to do it the complex way to avoid memory leaks
LVRef<LVColorDrawBuf> buf = LVRef<LVColorDrawBuf>( new LVColorDrawBuf(img_w, img_h, 32) );
buf->Draw(img, 0, 0, img_w, img_h, false); // (dither=false doesn't matter with a color buffer)
LVImageSourceRef src = LVCreateDrawBufImageSource(buf.get(), false);
LVImageSourceRef transformed = LVCreateStretchFilledTransform(src, transform_w, transform_h,
Expand Down

0 comments on commit 8254cef

Please sign in to comment.