New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 649739 link

Starred by 2 users

Issue metadata

Status: Archived
Owner:
Closed: Sep 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

Remove ICC profiles and gamma ramps from source tree

Project Member Reported by derat@chromium.org, Sep 23 2016

Issue description

Per https://chromium-review.googlesource.com/c/386584/, we're no longer dumping ICC profiles or gamma ramps into src/platform/assets. Rather, they should be added to the Quirks server and served through there.

Can we delete the old files from src/platform/assets so people don't get confused? Here the are with SHA1s:

d6f062e529469aad985f257c6a64bbc9bb856042  06af5c10.icc
5bff7dc5836bd42d57feedcf8f592fd293264d59  06af5c23.icc
4157b921933b85fba2251c1ac14c09fe6388c54e  alex.bin
4157b921933b85fba2251c1ac14c09fe6388c54e  lumpy.bin
2d965299e0f0ff93d6193d04247f0ac177f28826  mario.bin

Are the first two .icc files already on the Quirks server? Do we have ICC versions of the other three ramps? alex and mario are officially EOL-ed, I think, so maybe we can just drop those, but lumpy is still supported until May 2017 per https://support.google.com/chrome/a/answer/6220366?hl=en.
 

Comment 1 by sjoe@google.com, Sep 23 2016

Here are list of models supported by quirks server:
https://chromeosquirksserver-pa.googleapis.com/modellist/modellist.html

Yes, the first 2 icc files are already on Quirks server.

Comment 2 by derat@chromium.org, Sep 23 2016

Owner: derat@chromium.org
Status: Assigned (was: Available)
Heh. It looks like we only refer to the .bin files from boot-splash.conf:

    if [ -x /sbin/frecon ]; then
      frecon --daemon \
        --gamma /usr/share/color/bin/internal_display.bin \
        --clear 0xfffefefe $DEV_END_USER --frame-interval 25 \
        $BOOT_IMAGES
    else
      # Disable blinking cursor. Without this, a splash screen will show a
      # distinct cursor shape even when the cursor is set to none.
      echo 0 > /sys/devices/virtual/graphics/fbcon/cursor_blink

      ply-image --set-monitors \
        --gamma /usr/share/color/bin/internal_display.bin \
        --clear 0xfefefe --frame-interval 25 $BOOT_IMAGES
     fi

Which leads me to frecon's main.c:

  static const char * const command_help[] = {
  ...
          "The gamma table to apply. (unimplemented)",
  };

So I think these don't get used anymore... right?

Comment 3 by derat@chromium.org, Sep 23 2016

Status: Started (was: Assigned)
I'll remove the preinstalled files and then assign to Greg to clean up chrome/browser/chromeos/display/quirks_manager_delegate_impl.cc. :-)
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 24 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/dd8f28e1b30a62dfcbd9f44d85f420b99525da93

commit dd8f28e1b30a62dfcbd9f44d85f420b99525da93
Author: Daniel Erat <derat@chromium.org>
Date: Fri Sep 23 17:37:59 2016

common-assets: Stop installing ICC profiles and gamma ramps.

ICC profiles get fetched from the Quirks server (and the
two that we were installing are present there), and gamma
ramps are unimplemented in frecon.

BUG= chromium:649739 
TEST=none

Change-Id: I498c3e7993996f8afa6a74ce11482bafcf42b37a
Reviewed-on: https://chromium-review.googlesource.com/388758
Commit-Ready: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/dd8f28e1b30a62dfcbd9f44d85f420b99525da93/chromeos-base/common-assets/common-assets-9999.ebuild

Project Member

Comment 5 by bugdroid1@chromium.org, Sep 24 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/assets/+/9f8d3b10b907f2305e07507dae3ba5df0d90bac1

commit 9f8d3b10b907f2305e07507dae3ba5df0d90bac1
Author: Daniel Erat <derat@chromium.org>
Date: Fri Sep 23 17:39:43 2016

assets: Remove ICC profiles and gamma ramps.

ICC profiles get fetched from the Quirks server (and the
two that we were installing are present there), and gamma
ramps are unimplemented in frecon.

BUG= chromium:649739 
TEST=none
CQ-DEPEND=I498c3e7993996f8afa6a74ce11482bafcf42b37a

Change-Id: If03a75c569847f84db378c8c204fc1720c9dc979
Reviewed-on: https://chromium-review.googlesource.com/388782
Commit-Ready: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[delete] https://crrev.com/1db606f84a4b4b131cb0d480c4176d6e008360cd/color_profiles/lumpy.bin
[delete] https://crrev.com/1db606f84a4b4b131cb0d480c4176d6e008360cd/color_profiles/mario.bin
[delete] https://crrev.com/1db606f84a4b4b131cb0d480c4176d6e008360cd/color_profiles/06af5c10.icc
[delete] https://crrev.com/1db606f84a4b4b131cb0d480c4176d6e008360cd/color_profiles/06af5c23.icc
[delete] https://crrev.com/1db606f84a4b4b131cb0d480c4176d6e008360cd/color_profiles/alex.bin

Comment 6 by glevin@chromium.org, Sep 25 2016

As per Comment #3, I've created  Issue 650095  for the cleanup of related Quirks Client code.
Project Member

Comment 7 by bugdroid1@chromium.org, Sep 26 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/a9e3be8a70b780f598aa5939f618859c5c36752c

commit a9e3be8a70b780f598aa5939f618859c5c36752c
Author: Daniel Erat <derat@chromium.org>
Date: Fri Sep 23 17:41:31 2016

init: Stop setting gamma from boot-splash.

Remove the --gamma flag from our calls to frecon (which
doesn't implement it) and ply-image (which I don't think we
use anymore).

BUG= chromium:649739 
TEST=none

Change-Id: I59a5bd11b378028d3c6da83e1b8e8e0147a2dd6e
Reviewed-on: https://chromium-review.googlesource.com/388742
Commit-Ready: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/a9e3be8a70b780f598aa5939f618859c5c36752c/init/boot-splash.conf

Project Member

Comment 8 by bugdroid1@chromium.org, Sep 27 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/frecon/+/099685cafe4a5e0e310698cc6a76bed7a670f3e8

commit 099685cafe4a5e0e310698cc6a76bed7a670f3e8
Author: Daniel Erat <derat@chromium.org>
Date: Fri Sep 23 17:47:57 2016

frecon: Remove unimplemented -g/--gamma flag.

It doesn't work and we don't plan to use it.

BUG= chromium:649739 
TEST=built it
CQ-DEPEND=I59a5bd11b378028d3c6da83e1b8e8e0147a2dd6e

Change-Id: I3a6e343db8b43010165c1ab4f9a1ac5ac81ff2e4
Reviewed-on: https://chromium-review.googlesource.com/388815
Commit-Ready: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/099685cafe4a5e0e310698cc6a76bed7a670f3e8/main.c
[modify] https://crrev.com/099685cafe4a5e0e310698cc6a76bed7a670f3e8/README.md

Comment 9 by derat@chromium.org, Sep 27 2016

Status: Fixed (was: Started)
Labels: VerifyIn-55

Comment 11 by dchan@google.com, Nov 19 2016

Labels: VerifyIn-56
Project Member

Comment 12 by bugdroid1@chromium.org, Dec 23 2016

Labels: merge-merged-factory-gru-8652.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/5779c37962c43790b8a5ca782df359f286261162

commit 5779c37962c43790b8a5ca782df359f286261162
Author: Daniel Erat <derat@chromium.org>
Date: Fri Sep 23 17:41:31 2016

init: Stop setting gamma from boot-splash.

Remove the --gamma flag from our calls to frecon (which
doesn't implement it) and ply-image (which I don't think we
use anymore).

BUG= chromium:649739 
TEST=none

Change-Id: I59a5bd11b378028d3c6da83e1b8e8e0147a2dd6e
Reviewed-on: https://chromium-review.googlesource.com/388742
Commit-Ready: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/422818
Reviewed-by: Dan Erat <derat@chromium.org>
Commit-Queue: Jeffy Chen <jeffy.chen@rock-chips.com>
Tested-by: Jeffy Chen <jeffy.chen@rock-chips.com>

[modify] https://crrev.com/5779c37962c43790b8a5ca782df359f286261162/init/boot-splash.conf

Comment 13 by dchan@google.com, Jan 21 2017

Labels: VerifyIn-57

Comment 14 by dchan@google.com, Mar 4 2017

Labels: VerifyIn-58

Comment 15 by dchan@google.com, Apr 17 2017

Labels: VerifyIn-59

Comment 16 by dchan@google.com, May 30 2017

Labels: VerifyIn-60
Labels: VerifyIn-61

Comment 18 by dchan@chromium.org, Oct 14 2017

Status: Archived (was: Fixed)

Sign in to add a comment