The two comments show the same equation for alpha: Sa + Da - Sa*Da However, you make a great observation on the impl. I will update the code to use SkAlphaMulQ as you point out.
thanks, mike BTW - the most update skia sources are now at code.google.com/p/skia. Feel free to file bugs/requests directly on that site. I regularly pull from there back into Android (as other skia clients do too, like Chrome). On Tue, Mar 3, 2009 at 10:43 AM, Eric <[email protected]> wrote: > > Thanks, one more thing: > > in SkXfermode.cpp DstOver is defined as > > // kDstOver_Mode, //!< [Sa + (1 - Sa)*Da, Dc + (1 - Da)*Sc] > > Shouldn't the alpha part be: > > // kDstOver_Mode, //!< [Da + (1 - Da)*Sa, Dc + (1 - Da)*Sc] > > Making it exactly the same as SrcOver but with dst and src reversed > and then it can be defined as: > > static SkPMColor dstover_modeproc(SkPMColor src, SkPMColor dst) { > return dst + SkAlphaMulQ(src, 256 - SkGetPackedA32(dst)); > } > > Unless I am missing something? > > > On Mar 2, 9:44 pm, Mike Reed <[email protected]> wrote: >> Ah, if we only apply it to srcover. I'll take a wack at it. >> >> On Mon, Mar 2, 2009 at 10:22 AM, Eric <[email protected]> wrote: >> >> > It would indeed only require the following changes: >> >> > in SkColorPriv.h >> >> > inline SkPMColor SkPMSrcOver(SkPMColor src, SkPMColor dst) { >> > return src + SkAlphaMulQ(dst, 256 - SkGetPackedA32(src)); >> > } >> >> > and in SkXfermode.cpp >> >> > // kSrcOver_Mode, //!< [Sa + (1 - Sa)*Da, Sc + (1 - Sa)*Dc] >> > static SkPMColor srcover_modeproc(SkPMColor src, SkPMColor dst) { >> > return src + SkAlphaMulQ(dst, 256 - SkGetPackedA32(src)); >> > } >> >> > This would solve the "opaque pixel problem" and it is a bit faster. >> >> > On Feb 28, 12:07 am, pixelflinger <[email protected]> wrote: >> >> I think that Eric's point is that it works specifically for that >> >> case. >> >> The macro is correct in general, but in that specific case (32-bits, >> >> premult blending), a different global equation (with +1) would work >> >> better. >> >> It wouldn't cause artifacts, would be slightly faster and would have >> >> the nice property to never make a destination pixel less opaque. >> >> >> mathias >> >> >> On Feb 27, 12:50 pm, [email protected] wrote: >> >> >> > I have also tried the +1 rounding trick, but have seen overflows >> >> > (wrapping back to 0) in some modes, since we are asymmetrically >> >> > applying the add to only one of the alpha components. >> >> >> > On Feb 27, 10:22 am, Eric <[email protected]> wrote: >> >> >> > > For a great article about this please >> >> > > seehttp://www.stereopsis.com/doubleblend.html >> >> > > by Michael Herf (from Picasa) >> >> >> > > For the default blending function it describes an optimized version of >> >> > > the one Mike Reed mentions but also the one I referred to which I >> >> > > think is the fastest one possible. For this case it does satisfy the >> >> > > OpenGL criteria and I imagine for Android that speed is more >> >> > > important. >> >> >> > > On Feb 27, 2:05 pm, [email protected] wrote: >> >> >> > > > static inline U8CPU SkMulDiv255Round(U8CPU a, U8CPU b) { >> >> > > > SkASSERT((uint8_t)a == a); >> >> > > > SkASSERT((uint8_t)b == b); >> >> > > > unsigned prod = SkMulS16(a, b) + 128; >> >> > > > return (prod + (prod >> 8)) >> 8; >> >> >> > > > } >> >> >> > > > This is used in some places for blending. I think it has even >> >> > > > smaller >> >> > > > error than (a + (a >> 7)), though it is slower. >> >> >> > > > On Feb 27, 6:29 am, pixelflinger <[email protected]> wrote: >> >> >> > > > > The problem in the code you pointed to can be improved by >> >> > > > > rounding the >> >> > > > > result (add 0x80 before shifting by 8). >> >> >> > > > > result = S + D * (((256 - (A + (A>>7))) + 0x80) >> 8) >> >> >> > > > > What you're proposing is interesting, essentially: >> >> >> > > > > result = S + (D * (256 - A))>>8 >> >> >> > > > > Basically, this has a bigger error, but it is biased on the >> >> > > > > "opaque" >> >> > > > > side, so it never causes a pixel to become more transparent. It >> >> > > > > looks >> >> > > > > like it works very well for the premultiplied alpha case above. >> >> > > > > I'll >> >> > > > > put this on the list of stuff to investigate further :-) >> >> > > > > Thanks for the feedback. >> >> >> > > > > Mathias >> >> >> > > > > On Feb 27, 1:56 am, Eric <[email protected]> wrote: >> >> >> > > > > > Correct, that is why I mentioned it cannot be used in all >> >> > > > > > cases. The >> >> > > > > > fact is that for some PorterDuff modes (see the SrcOver example >> >> > > > > > above ) the current implementation with the shift optimization >> >> > > > > > produces incorrect results because fully opaque pixels (with >> >> > > > > > alpha >> >> > > > > > 255) that should remain fully opaque turn into partially >> >> > > > > > transparent >> >> > > > > > pixels (with alpha 254). It is only a very slight difference >> >> > > > > > but it >> >> > > > > > can be avoided. >> >> >> > > > > > On Feb 27, 8:52 am, pixelflinger <[email protected]> >> >> > > > > > wrote: >> >> >> > > > > > > Hello, >> >> >> > > > > > > If you compute the errors, that is: >> >> >> > > > > > > a*float(256/255) - (a + (a>>7)) >> >> >> > > > > > > and >> >> >> > > > > > > a*float(256/255) - (a + 1) >> >> >> > > > > > > you'll see that the former is a better choice; the maximum >> >> > > > > > > error is >> >> > > > > > > smaller (0.5 instead of 1.0) >> >> >> > > > > > > The later is also incorrect per the OpenGL blending >> >> > > > > > > specification >> >> > > > > > > because it doesn't map "0" to "0" (you're lucky in that >> >> > > > > > > specific >> >> > > > > > > example, but if you were to dither the result, it wouldn't >> >> > > > > > > work). >> >> >> > > > > > > Additionally, doing the actual computation in integer: >> >> >> > > > > > > (a * 256)/255 >> >> >> > > > > > > will give you the same result than with the equation used in >> >> > > > > > > skia. >> >> >> > > > > > > Mathias >> >> >> > > > > > > On Feb 26, 11:04 am, Eric <[email protected]> wrote: >> >> >> > > > > > > > When using some of the PorterDuff modes in the Android SDK >> >> > > > > > > > incorrect >> >> > > > > > > > alpha values are produced. For example the following: >> >> >> > > > > > > > Bitmap bitmap = Bitmap.createBitmap(100, 100, >> >> > > > > > > > Bitmap.Config.ARGB_8888); >> >> > > > > > > > bitmap.eraseColor(0xff000000); // Black with alpha 255 >> >> > > > > > > > Canvas canvas = new Canvas(bitmap); >> >> > > > > > > > canvas.drawColor(0x80000000, PorterDuff.Mode.SRC_OVER); // >> >> > > > > > > > Black with >> >> > > > > > > > alpha 128 >> >> >> > > > > > > > results in 0xfe000000 so an alpha value of 254 instead of >> >> > > > > > > > 255 >> >> >> > > > > > > > After some digging in the source code I traced it back to >> >> > > > > > > > incorrect >> >> > > > > > > > use of SkAlpha255To256 defined in >> >> >> > > > > > > > platform/external/skia.git/include/core/SkColorPriv.h >> >> >> > > > > > > > 34 static inline unsigned SkAlpha255To256(U8CPU alpha) { >> >> > > > > > > > 35 SkASSERT(SkToU8(alpha) == alpha); >> >> > > > > > > > 36 return alpha + (alpha >> 7); >> >> > > > > > > > 37 } >> >> >> > > > > > > > It is frequently used in the PorterDuff blending functions, >> >> > > > > > > > below is >> >> > > > > > > > SRC_OVER from: >> >> >> > > > > > > > platform/external/skia.git/src/core/SkXfermode.cpp >> >> >> > > > > > > > 347 // kSrcOver_Mode, //!< [Sa + (1 - Sa)*Da, Sc + (1 - >> >> > > > > > > > Sa)*Dc] >> >> > > > > > > > 348 static SkPMColor srcover_modeproc(SkPMColor src, >> >> > > > > > > > SkPMColor dst) { >> >> > > > > > > > 349 return src + SkAlphaMulQ(dst, SkAlpha255To256(255 - >> >> > > > > > > > SkGetPackedA32(src))); >> >> > > > > > > > 350 } >> >> >> > > > > > > > Running the above numbers for alpha it produces 128 + (255 >> >> > > > > > > > * 127) >> 8 >> >> > > > > > > > = 254. Doing it without the shift optimization returns 128 >> >> > > > > > > > + (255 * >> >> > > > > > > > 127) / 255 = 255. >> >> >> > > > > > > > Fro some of these functions (not all) it would be better to >> >> > > > > > > > use a >> >> > > > > > > > SkAlpha255To256 defined as >> >> >> > > > > > > > static inline unsigned SkAlpha255To256(U8CPU alpha) { >> >> > > > > > > > SkASSERT(SkToU8(alpha) == alpha); >> >> > > > > > > > return alpha + 1; >> >> >> > > > > > > > } >> >> >> > > > > > > > which is not 100% accurate either but you can still use the >> >> > > > > > > > shift >> >> > > > > > > > trick and it does produce correct result when alpha is 0 >> >> > > > > > > > and when >> >> > > > > > > > alpha is 255. Any thoughts? >> >> > > > --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "android-framework" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/android-framework?hl=en -~----------~----~----~----~------~----~------~--~---
