Adjust maximum decoded image size on Android

Adjust maximum decoded image size on Android

The maximum decoded image size is adjusted from 1/32 of accessible
memory to 1/25 of accessible memory. This is to enable the upload of
4K*4K WebGL textures on platforms with 2GB of physical memory of which a
small region is reserved for direct hardware access and thus invisible to
Chromium.

BUG=356181
R=kbr@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=261371

[piman, 2014-04-02 20:00:18]

LGTM owner stamp, but +aelias for double-checking the assumptions here.

[aelias_OOO_until_Jul13, 2014-04-02 21:23:46]

[+wangxianzhu] who added this limit in the first place.

The linked bug for what this is fixing is restricted (can you remove that
restriction?) and it's not clear to me from the original bug
(https://code.google.com/p/chromium/issues/detail?id=303848 ) why this limit was
added at all.  So I'm unclear on the practical tradeoffs here.

In principle though, does it even make sense to have a per-image size limit that
depends on the system memory?  Shouldn't system memory only be used as an
overall budget for all images?

[Ken Russell (switch to Gerrit), 2014-04-02 21:24:41]

I'm not qualified to review this patch but it looks fine.

[piman, 2014-04-02 21:36:56]

On Wed, Apr 2, 2014 at 2:23 PM, <aelias@chromium.org> wrote:

> [+wangxianzhu] who added this limit in the first place.
>
> The linked bug for what this is fixing is restricted (can you remove that
> restriction?)


I removed the restriction on the bug.


> and it's not clear to me from the original bug
> (https://code.google.com/p/chromium/issues/detail?id=303848 ) why this
> limit was
> added at all.  So I'm unclear on the practical tradeoffs here.
>
> In principle though, does it even make sense to have a per-image size
> limit that
> depends on the system memory?  Shouldn't system memory only be used as an
> overall budget for all images?
>
> https://codereview.chromium.org/220993003/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Xianzhu, 2014-04-02 21:38:14]

lgtm.

The original limit was just to avoid immediate crash when decoding a single very
huge image.

I think we have overall budget management for all decoded images.

[aelias_OOO_until_Jul13, 2014-04-02 22:10:49]

On 2014/04/02 21:38:14, Xianzhu wrote:
> lgtm.
> 
> The original limit was just to avoid immediate crash when decoding a single
very
> huge image.
> 
> I think we have overall budget management for all decoded images.

OK.  For predictability, maybe we can replace the limit with a fixed number like
4096*4096*4 ?

[Xianzhu, 2014-04-02 22:22:00]

On 2014/04/02 22:10:49, aelias wrote:
> On 2014/04/02 21:38:14, Xianzhu wrote:
> > lgtm.
> > 
> > The original limit was just to avoid immediate crash when decoding a single
> very
> > huge image.
> > 
> > I think we have overall budget management for all decoded images.
> 
> OK.  For predictability, maybe we can replace the limit with a fixed number
like
> 4096*4096*4 ?

I can't access bug 356181. What's the bug about? Why is 4Kx4K-texture relevant?

[Xianzhu, 2014-04-03 01:54:48]

Lg'ed because I thought the changed number would work just like the original
number.

However, how is the max decoded image size related to textures? I didn't see the
relationship from the bug and the fix.

[oetuaho-nv, 2014-04-03 08:58:01]

On 2014/04/03 01:54:48, Xianzhu wrote:
> Lg'ed because I thought the changed number would work just like the original
> number.
> 
> However, how is the max decoded image size related to textures? I didn't see
the
> relationship from the bug and the fix.

WebGL texImage2D that takes in a bitmap HTMLImageElement is also subject to this
size restriction. That's what caused the bug.

I'd like to look further into removing this limit altogether in favor of an
overall decoded image budget, but I'm committing this change to get something
done sooner.

I would like to avoid having a fixed limit, since the code is going to be used
on a wide variety of devices. A one-size-fits-all future is not in sight at
least yet.

[oetuaho-nv, 2014-04-03 08:58:10]

The CQ bit was checked by oetuaho@nvidia.com

[commit-bot: I haz the power, 2014-04-03 08:59:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/oetuaho@nvidia.com/220993003/1

[commit-bot: I haz the power, 2014-04-03 12:21:35]

Change committed as 261371