TODO work items in image_skia.cc |
||||
Issue description
UserAgent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:55.0) Gecko/20100101 Firefox/55.0
Steps to reproduce the problem:
This is regarding TODO workitmes in image_skia.cc
void ImageSkia::Init(const ImageSkiaRep& image_rep) {
// TODO(pkotwicz): The image should be null whenever image rep is null.
if (image_rep.sk_bitmap().empty()) {
storage_ = NULL;
return;
}
storage_ = new internal::ImageSkiaStorage(
NULL, gfx::Size(image_rep.GetWidth(), image_rep.GetHeight()));
storage_->image_reps().push_back(image_rep);
}
const SkBitmap& ImageSkia::GetBitmap() const {
if (isNull()) {
// Callers expect a ImageSkiaRep even if it is |isNull()|.
// TODO(pkotwicz): Fix this.
return NullImageRep().sk_bitmap();
}
// TODO(oshima): This made a few tests flaky on Windows.
// Fix the root cause and re-enable this. crbug.com/145623 .
#if !defined(OS_WIN)
CHECK(CanRead());
#endif
ImageSkiaReps::iterator it = storage_->FindRepresentation(1.0f, true);
if (it != storage_->image_reps().end())
return it->sk_bitmap();
return NullImageRep().sk_bitmap();
}
What is the expected behavior?
What went wrong?
NA
Did this work before? N/A
Chrome version: 61.0.3136.100 Channel: n/a
OS Version:
Flash Version:
@pkotwicz,
I saw TODO comments in image_skia.cc, could you please give bit more details about this.
I am interested to work on this
,
Oct 31 2017
Seems it is a feature request marking it as Untraiged to get more inputs from dev. Could some one from dev team please take a look into it. Thanks..!
,
Nov 1 2017
,
Nov 2 2017
The TODO in ImageSkia::Init() refers to changing if (image_rep.sk_bitmap().empty()) to if (image_rep.sk_bitmap().drawsNothing()) I did not do this initially because doing this broke tests (I unfortunately don't remember which tests broke) The first step to addressing the TODO in ImageSkia::GetBitmap() is to convert all of the usages of Image::ToSkBitmap() to Image::AsBitmap() I don't recommend addressing the second TODO because it is a lot of work for not much gain
,
Nov 2 2017
I shall try to replace as below and check if any failures in tests?
void ImageSkia::Init(const ImageSkiaRep& image_rep) {
// TODO(pkotwicz): The image should be null whenever image rep is null.
if (image_rep.sk_bitmap().drawsNothing()) {
storage_ = NULL;
return;
}
storage_ = new internal::ImageSkiaStorage(
NULL, gfx::Size(image_rep.GetWidth(), image_rep.GetHeight()));
storage_->image_reps().push_back(image_rep);
}
,
Nov 2 2017
Uploaded below patch. PTAL https://chromium-review.googlesource.com/c/chromium/src/+/750484
,
Nov 7 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7aad9de9abf65f559ffe2f9170c534ef6c9e0b7a commit 7aad9de9abf65f559ffe2f9170c534ef6c9e0b7a Author: cm.sanchi <cm.sanchi@samsung.com> Date: Tue Nov 07 05:23:37 2017 Replaced sk_bitmap().empty() with drawsNothing() Replaced sk_bitmap().empty() with drawsNothing(). drawsNothing function return true if width() or height() are zero, or if SkPixelRef is nullptr that means drawing has no effect BUG= 779516 Change-Id: I7cf908e2faf215854b12277d09f3572322ceb670 Reviewed-on: https://chromium-review.googlesource.com/750484 Commit-Queue: srirama chandra sekhar <srirama.m@samsung.com> Reviewed-by: Peter Kotwicz <pkotwicz@chromium.org> Reviewed-by: Mitsuru Oshima <oshima@chromium.org> Cr-Commit-Position: refs/heads/master@{#514403} [modify] https://crrev.com/7aad9de9abf65f559ffe2f9170c534ef6c9e0b7a/ui/gfx/image/image_skia.cc
,
Dec 8 2017
|
||||
►
Sign in to add a comment |
||||
Comment 1 by satay...@samsung.com
, Oct 30 2017