Done. btw - interesting that this reduces to x == x * 255 / 256; // for x: 0..255
On Tue, Mar 3, 2009 at 2:18 PM, Mike Reed <[email protected]> wrote: > 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 -~----------~----~----~----~------~----~------~--~---
