Open Bug 1043745 Opened 8 years ago Updated 7 years ago

[Skia] Stop using SkBitmap::Config

Categories

(Core :: Graphics, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

People

(Reporter: gw280, Unassigned)

References

Details

Attachments

(1 file)

Upstream removed the notion of SkBitmap 'Configs' (roughly equivalent to Moz2D's 'Format').
It seems the new descriptor is called SkColorType
Blocks: 1017113
Comment on attachment 8469016 [details] [diff] [review]
0001-Bug-1043745-Use-SkColorType-and-SkImageInfo-instead-.patch

Review of attachment 8469016 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/2d/DrawTargetSkia.cpp
@@ +114,5 @@
> +  SkImageInfo info = SkImageInfo::Make(surf->GetSize().width,
> +                                       surf->GetSize().height,
> +                                       GfxFormatToSkiaColorType(surf->GetFormat()),
> +                                       alphaType);
> +  result.mBitmap.setInfo(info);

You need to pass surf->Stride() for the second (optional, but not for us) parameter.

@@ +584,5 @@
>  
>      SkMatrix transform = maskPaint.getShader()->getLocalMatrix();
>      transform.postTranslate(SkFloatToScalar(aOffset.x), SkFloatToScalar(aOffset.y));
> +    SkShader* matrixShader = SkShader::CreateLocalMatrixShader(maskPaint.getShader(), transform);
> +    maskPaint.setShader(matrixShader);

Don't we need to unref the return value here too?

@@ +823,5 @@
> +  SkImageInfo info = SkImageInfo::Make(aSize.width,
> +                                       aSize.height,
> +                                       GfxFormatToSkiaColorType(aFormat),
> +                                       alphaType);
> +  bitmap.setInfo(info);

Stride!

::: gfx/2d/HelpersSkia.h
@@ +38,5 @@
>    }
>  }
>  
> +static inline SurfaceFormat
> +SkiaConfigToGfxFormat(SkBitmap::Config config)

Are either of these actually used? I don't see it.

::: gfx/2d/SourceSurfaceSkia.cpp
@@ +71,5 @@
> +  SkImageInfo info = SkImageInfo::Make(aSize.width,
> +                                       aSize.height,
> +                                       GfxFormatToSkiaColorType(aFormat),
> +                                       alphaType);
> +  temp.setInfo(info);

oh, the poor strides

::: other-licenses/skia-npapi/SkANP.cpp
@@ +69,5 @@
>              break;
>      }
>      
> +    SkImageInfo info = SkImageInfo::Make(src.width, src.height, colorType, kPremul_SkAlphaType);
> +    dst->setInfo(info, src.rowBytes);

stride success!
Attachment #8469016 - Flags: review?(matt.woodrow) → review+
Good to see you're taking my errors in stride.
... this was landed before the skia update, which means it broke the build. That didn't happen on tbpl because it was landed in the same push, but it brakes e.g. bisection.
Oops. That was unfortunate. Sorry.
You need to log in before you can comment on or make changes to this bug.