Closed Bug 777614 Opened 13 years ago Closed 13 years ago

[Skia] Do another rebase

Categories

(Core :: Graphics, defect)

15 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla18

People

(Reporter: nrc, Assigned: gw280)

References

Details

Attachments

(8 files)

1.12 KB, patch
nrc
: review+
Details | Diff | Splinter Review
1.15 KB, patch
nrc
: review+
Details | Diff | Splinter Review
21.26 KB, patch
nrc
: review+
Details | Diff | Splinter Review
12.43 KB, patch
nrc
: review+
Details | Diff | Splinter Review
2.44 KB, patch
nrc
: review+
Details | Diff | Splinter Review
2.81 KB, patch
nrc
: review+
Details | Diff | Splinter Review
1.05 KB, patch
nrc
: review+
Details | Diff | Splinter Review
1.51 KB, patch
nrc
: review+
Details | Diff | Splinter Review
Apparently they've updated their gradient code, and we need that for bug 718849, which is blocking turning on Azure/Skia canvas, and that makes kittens cry.
Stop making kittens cry, you monster.
Depends on: 777615
Attachment #661094 - Flags: review?(ncameron) → review+
Comment on attachment 661094 [details] [diff] [review] Bug 777614 - Re-add our SkUserConfig.h Review of attachment 661094 [details] [diff] [review]: ----------------------------------------------------------------- Do you need to update the patch which does this too, or is that somewhere else?
Attachment #661095 - Flags: review?(ncameron) → review+
Attachment #661098 - Flags: review?(ncameron) → review+
Attachment #661100 - Flags: review?(ncameron) → review+
Attachment #661101 - Flags: review?(ncameron) → review+
Comment on attachment 661099 [details] [diff] [review] Bug 777614 - Handle BGRX SourceSurfaces in Skia by converting them to BGRA Review of attachment 661099 [details] [diff] [review]: ----------------------------------------------------------------- r+ with the casts changed ::: gfx/2d/HelpersSkia.h @@ +115,5 @@ > > +static inline void > +ConvertBGRXToBGRA(unsigned char* aData, const IntSize &aSize, int32_t aStride) > +{ > + uint32_t* pixel = (uint32_t*)aData; nit: use reinterpret_cast @@ +121,5 @@ > + for (int row = 0; row < aSize.height; ++row) { > + for (int column = 0; column < aSize.width; ++column) { > + pixel[column] |= 0xFF000000; > + } > + pixel += (aStride/4); nit: brackets unnecessary, but I don't really object to leaving them in ::: gfx/2d/SourceSurfaceSkia.cpp @@ +52,5 @@ > + > + if (aFormat == FORMAT_B8G8R8X8) { > + mBitmap.lockPixels(); > + // We have to manually set the A channel to be 255 as Skia doesn't understand BGRX > + ConvertBGRXToBGRA((unsigned char*)mBitmap.getPixels(), aSize, aStride); reinterpret_cast
Attachment #661099 - Flags: review?(ncameron) → review+
Comment on attachment 661097 [details] [diff] [review] Bug 777614 - Re-apply bug 687188 - Expand the gradient cache by 2 to store 0/1 colour stop values for clamping. Review of attachment 661097 [details] [diff] [review]: ----------------------------------------------------------------- lgtm, but I don't deeply understand the code, I assume it has been reviewed before, given the name of the patch, if not we should ask someone who knows more for a review too. Why hasn't this been upstreamed? ::: gfx/skia/src/effects/gradients/SkGradientShader.cpp @@ +478,5 @@ > + SkColorGetB(fOrigColors[0])); > + fCache32[kCache32ClampUpper] = SkPackARGB32(fCacheAlpha, SkColorGetR(fOrigColors[fColorCount - 1]), > + SkColorGetG(fOrigColors[fColorCount - 1]), > + SkColorGetB(fOrigColors[fColorCount - 1])); > + nit: new lines after fCacheAlpha
Attachment #661097 - Flags: review?(ncameron) → review+
Comment on attachment 661096 [details] [diff] [review] Bug 777614 - Re-apply bug 719872 - Fix crash on Android by reverting to older FontHost Review of attachment 661096 [details] [diff] [review]: ----------------------------------------------------------------- Ugh, really? Is there not a better fix than reverting to some old code? Like fixing the new code? Do we not need to change a makefile or something to use this file rather than the original, or does that stay the same independent of the rebase? To clarify, are we just substituting older Skia code for newer Skia code here, we're not adding anything else etc.? I would prefer if that was apparent from the patch, for e.g., by changing the original file rather than adding a new one.
We already have this revert in the current skia that's in the tree; I'm just re-applying it. The "correct" fix is to use my Cairo SkFontHost, but that's not quite ready yet.
(In reply to Nick Cameron [:nrc] from comment #12) > Comment on attachment 661097 [details] [diff] [review] > Bug 777614 - Re-apply bug 687188 - Expand the gradient cache by 2 to store > 0/1 colour stop values for clamping. > > Review of attachment 661097 [details] [diff] [review]: > ----------------------------------------------------------------- > > lgtm, but I don't deeply understand the code, I assume it has been reviewed > before, given the name of the patch, if not we should ask someone who knows > more for a review too. > > Why hasn't this been upstreamed? I'm working on it, but it's taking a while, so this version is going into m-c for now until I can get it upstreamed.
Comment on attachment 661096 [details] [diff] [review] Bug 777614 - Re-apply bug 719872 - Fix crash on Android by reverting to older FontHost Review of attachment 661096 [details] [diff] [review]: ----------------------------------------------------------------- Ugh, really? Is there not a better fix than reverting to some old code? Like fixing the new code? Do we not need to change a makefile or something to use this file rather than the original, or does that stay the same independent of the rebase? To clarify, are we just substituting older Skia code for newer Skia code here, we're not adding anything else etc.? I would prefer if that was apparent from the patch, for e.g., by changing the original file rather than adding a new one.
Attachment #661096 - Flags: review?(ncameron) → review+
Depends on: 901208
Was the removal of gfx/skia/update.sh intentional?
Yes. It was hard coded and very much out of date.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: