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

Issue 764007 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Sep 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

FastInk broken on caroline

Project Member Reported by kaznacheev@chromium.org, Sep 11 2017

Issue description

First observed on ChromeOS R62-9880.0.0, Chrome 62.0.3197.0

Steps:
1. Enable "Laser pointer" stylus tool
2. Start drawing

Expected: the pointer is following the stylus tip
Actual: the pointer is drawn and updates a few times, then stays in place

Does not happen in M61

 
Cc: fsam...@chromium.org reve...@chromium.org
Owner: kaznacheev@chromium.org
Status: Available (was: Untriaged)
https://chromium.googlesource.com/chromium/src/+/c6235344869942ceeaddc4b788625ac7f71df258%5E%21/ might be the cause as the other fast ink changes I see was introduced in that version have been merged to 61.

kaznacheev@, can you reproduce this with a cros build on your workstation?
I tried, but for some reason I cannot get mouse events to get to the laser pointer's event handler.
You have to disable this check (we should add a debug flag so we don't have to change the code for this type of testing): https://cs.chromium.org/chromium/src/ash/fast_ink/fast_ink_pointer_controller.cc?l=68

and you need to emulate touch device using this flag: https://cs.chromium.org/chromium/src/ui/events/event_switches.cc?l=24
Thanks! This is the stack trace I am getting on the first touch event.

[17810:17810:0911/171159.693152:FATAL:render_pass.cc(175)] Check failed: damage_rect.IsEmpty() || output_rect.Contains(damage_rect). damage_rect: -6,-6 12x12 output_rect: 0,0 1366x768
#0 0x7f0c8273944d base::debug::StackTrace::StackTrace()
#1 0x7f0c827379cc base::debug::StackTrace::StackTrace()
#2 0x7f0c827c176d logging::LogMessage::~LogMessage()
#3 0x7f0c7a5a4e5f cc::RenderPass::SetNew()
#4 0x7f0c73e69d7d ash::FastInkView::UpdateSurface()
#5 0x7f0c73e6843d ash::FastInkView::UpdateBuffer()
#6 0x7f0c73e10f6f _ZN4base8internal13FunctorTraitsIMN3ash23DisplayAnimatorChromeOSEFvvEvE6InvokeIRKNS_7WeakPtrIS3_EEJEEEvS5_OT_DpOT0_
#7 0x7f0c73e6d64a _ZN4base8internal12InvokeHelperILb1EvE8MakeItSoIRKMN3ash11FastInkViewEFvvERKNS_7WeakPtrIS5_EEJEEEvOT_OT0_DpOT1_
#8 0x7f0c73e6d5e0 _ZN4base8internal7InvokerINS0_9BindStateIMN3ash11FastInkViewEFvvEJNS_7WeakPtrIS4_EEEEEFvvEE7RunImplIRKS6_RKNSt3__15tupleIJS8_EEEJLm0EEEEvOT_OT0_NSF_16integer_sequenceImJXspT1_EEEE
#9 0x7f0c73e6d52c _ZN4base8internal7InvokerINS0_9BindStateIMN3ash11FastInkViewEFvvEJNS_7WeakPtrIS4_EEEEEFvvEE3RunEPNS0_13BindStateBaseE
#10 0x7f0c826e7521 _ZNO4base12OnceCallbackIFvvEE3RunEv
#11 0x7f0c8273dda8 base::debug::TaskAnnotator::RunTask()
#12 0x7f0c827e5fef base::internal::IncomingTaskQueue::RunTask()
#13 0x7f0c827eacd6 base::MessageLoop::RunTask()
#14 0x7f0c827eaf37 base::MessageLoop::DeferOrRunPendingTask()
#15 0x7f0c827ebc20 base::MessageLoop::DoWork()
#16 0x7f0c827f40a3 base::MessagePumpLibevent::Run()
#17 0x7f0c827ea586 base::MessageLoop::Run()
#18 0x7f0c8289c097 base::RunLoop::Run()
#19 0x56137c612e15 ChromeBrowserMainParts::MainMessageLoopRun()
#20 0x7f0c7c148f0b content::BrowserMainLoop::RunMainMessageLoopParts()
#21 0x7f0c7c150bc5 content::BrowserMainRunnerImpl::Run()
#22 0x7f0c7c13d24b content::BrowserMain()
#23 0x7f0c7dcb6b19 content::RunNamedProcessTypeMain()
#24 0x7f0c7dcb95a6 content::ContentMainRunnerImpl::Run()
#25 0x7f0c7dcb442d content::ContentServiceManagerMainDelegate::RunEmbedderProcess()
#26 0x7f0c83030f4c service_manager::Main()
#27 0x7f0c7dcb5a6b content::ContentMain()
#28 0x561379de2124 ChromeMain
#29 0x561379de1fe2 main
#30 0x7f0c675f0f45 __libc_start_main
#31 0x561379de1ec4 <unknown>

Ok, that's bad. Initial damage should match the output size and if it's ever possible for damage to be larger than bounds of output rect then we should clamp that.
Reporting back: force-clipped the damage to fit the output, seeing the laser pointer following the mouse pointer reasonably well (prediction is very irregular, but that's another story).
Yes, prediction is not working well with touch or mouse events as input. I'm hoping to fix that when upgrading to a better prediction filter.

Is the damage we collect correct? Clamping it sgtm unless there's something wrong with our damage tracking code.
It actually does not. There is quite a few updates with origin (-6,-6) which I think should not be happening. I am traveling for the next two, can resume looking on Thursday.
Thanks. I'm in a similar situation until Thursday so this might have to wait until then to get solved.
Last update: the same patch (with clamping) on an actual device does not solve anything.
Cc: kaznacheev@chromium.org
Labels: M-61
Owner: reve...@chromium.org
Status: Started (was: Available)
Project Member

Comment 12 by bugdroid1@chromium.org, Sep 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/545a726cfc197acd0c3e3ce28b5fedf3c63983c6

commit 545a726cfc197acd0c3e3ce28b5fedf3c63983c6
Author: David Reveman <reveman@chromium.org>
Date: Fri Sep 15 22:32:04 2017

ash: Fast ink damage fixes.

Limit damage rectangle to output rectangle and fix stationary point
generation and bounding box calculations for laser pointer to
make sure damage is reported correctly.

Bug:  764007 
Test: manual
Change-Id: Ie6352d539391776aa99ee00b7d1d302f0249e3dc
Reviewed-on: https://chromium-review.googlesource.com/668663
Commit-Queue: David Reveman <reveman@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Vladislav Kaznacheev <kaznacheev@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502408}
[modify] https://crrev.com/545a726cfc197acd0c3e3ce28b5fedf3c63983c6/ash/fast_ink/fast_ink_view.cc
[modify] https://crrev.com/545a726cfc197acd0c3e3ce28b5fedf3c63983c6/ash/laser/laser_pointer_view.cc
[modify] https://crrev.com/545a726cfc197acd0c3e3ce28b5fedf3c63983c6/ash/laser/laser_pointer_view.h

Project Member

Comment 13 by bugdroid1@chromium.org, Sep 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/906a31b9533e6e700602008efab3f917cec4b170

commit 906a31b9533e6e700602008efab3f917cec4b170
Author: David Reveman <reveman@chromium.org>
Date: Fri Sep 15 22:47:10 2017

ash: Handle exported fast ink textures correctly.

Exported textures should be considered lost when frame sink
is destroyed to prevent them from being deleted when still
in use.

This also removes the FastInkLayerTreeFrameSinkHolder class,
which is not needed and just adds an unnecessary layer of
indirect.

Bug:  764007 
Test: manual
Change-Id: Idf2dd68e846cb70d821079b1873a02ec7b2a93e3
Reviewed-on: https://chromium-review.googlesource.com/669779
Commit-Queue: David Reveman <reveman@chromium.org>
Reviewed-by: Vladislav Kaznacheev <kaznacheev@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502417}
[modify] https://crrev.com/906a31b9533e6e700602008efab3f917cec4b170/ash/fast_ink/fast_ink_view.cc
[modify] https://crrev.com/906a31b9533e6e700602008efab3f917cec4b170/ash/fast_ink/fast_ink_view.h

Labels: Merge-Request-61 Merge-Request-62
Status: Fixed (was: Started)
Project Member

Comment 15 by sheriffbot@chromium.org, Sep 16 2017

Labels: -Merge-Request-62 Merge-Review-62 Hotlist-Merge-Review
This bug requires manual review: M62 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 16 by bugdroid1@chromium.org, Sep 17 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/869d13439f5712193f27ded17a6300b67fd318f3

commit 869d13439f5712193f27ded17a6300b67fd318f3
Author: David Reveman <reveman@chromium.org>
Date: Sun Sep 17 00:29:14 2017

ash: Fix exported fast ink texture leak.

Exported textures needs to be delete to not leak memory. This
adds an ExportedTextureDeleter helper class that can be used
to delete exported textures while still making sure the
compositor had a chance to consume them.

Bug:  764007 
Test: manual
Change-Id: I18fa0494282f2352e73b320c05add8d944615213
Reviewed-on: https://chromium-review.googlesource.com/669586
Commit-Queue: David Reveman <reveman@chromium.org>
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502518}
[modify] https://crrev.com/869d13439f5712193f27ded17a6300b67fd318f3/ash/fast_ink/fast_ink_view.cc
[modify] https://crrev.com/869d13439f5712193f27ded17a6300b67fd318f3/ash/fast_ink/fast_ink_view.h

Labels: -Hotlist-Merge-Review -Merge-Review-62 Merge-Approved-62
Merge approved for 62.

The initial bug says this does not happen in 61, so why merge to 61?
Labels: -M-61 -Merge-Request-61
The black flashes is an issue in 61 too but that is not as bad as the damage rect problem that is broken in 62. I think it's fine to only fix this in 62. Removing 61 merge request.
Project Member

Comment 19 by bugdroid1@chromium.org, Sep 18 2017

Labels: -merge-approved-62 merge-merged-3202
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cb1bf5aebba7cada590a33ef34ab68d19ad1bbfe

commit cb1bf5aebba7cada590a33ef34ab68d19ad1bbfe
Author: David Reveman <reveman@chromium.org>
Date: Mon Sep 18 21:37:42 2017

ash: Fast ink damage fixes.

Limit damage rectangle to output rectangle and fix stationary point
generation and bounding box calculations for laser pointer to
make sure damage is reported correctly.

TBR=reveman@chromium.org

(cherry picked from commit 545a726cfc197acd0c3e3ce28b5fedf3c63983c6)

Bug:  764007 
Test: manual
Change-Id: Ie6352d539391776aa99ee00b7d1d302f0249e3dc
Reviewed-on: https://chromium-review.googlesource.com/668663
Commit-Queue: David Reveman <reveman@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Vladislav Kaznacheev <kaznacheev@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#502408}
Reviewed-on: https://chromium-review.googlesource.com/671868
Reviewed-by: David Reveman <reveman@chromium.org>
Cr-Commit-Position: refs/branch-heads/3202@{#309}
Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098}
[modify] https://crrev.com/cb1bf5aebba7cada590a33ef34ab68d19ad1bbfe/ash/fast_ink/fast_ink_view.cc
[modify] https://crrev.com/cb1bf5aebba7cada590a33ef34ab68d19ad1bbfe/ash/laser/laser_pointer_view.cc
[modify] https://crrev.com/cb1bf5aebba7cada590a33ef34ab68d19ad1bbfe/ash/laser/laser_pointer_view.h

Project Member

Comment 20 by bugdroid1@chromium.org, Sep 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ac0bf9c54b058a3818885a2f467c3559a85bf11e

commit ac0bf9c54b058a3818885a2f467c3559a85bf11e
Author: David Reveman <reveman@chromium.org>
Date: Mon Sep 18 21:49:20 2017

ash: Handle exported fast ink textures correctly.

Exported textures should be considered lost when frame sink
is destroyed to prevent them from being deleted when still
in use.

This also removes the FastInkLayerTreeFrameSinkHolder class,
which is not needed and just adds an unnecessary layer of
indirect.

TBR=reveman@chromium.org

(cherry picked from commit 906a31b9533e6e700602008efab3f917cec4b170)

Bug:  764007 
Test: manual
Change-Id: Idf2dd68e846cb70d821079b1873a02ec7b2a93e3
Reviewed-on: https://chromium-review.googlesource.com/669779
Commit-Queue: David Reveman <reveman@chromium.org>
Reviewed-by: Vladislav Kaznacheev <kaznacheev@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#502417}
Reviewed-on: https://chromium-review.googlesource.com/671870
Reviewed-by: David Reveman <reveman@chromium.org>
Cr-Commit-Position: refs/branch-heads/3202@{#310}
Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098}
[modify] https://crrev.com/ac0bf9c54b058a3818885a2f467c3559a85bf11e/ash/fast_ink/fast_ink_view.cc
[modify] https://crrev.com/ac0bf9c54b058a3818885a2f467c3559a85bf11e/ash/fast_ink/fast_ink_view.h

Project Member

Comment 21 by bugdroid1@chromium.org, Sep 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/032a160140f7b56a4066862f4be1cfa84de22c90

commit 032a160140f7b56a4066862f4be1cfa84de22c90
Author: David Reveman <reveman@chromium.org>
Date: Mon Sep 18 21:57:19 2017

ash: Fix exported fast ink texture leak.

Exported textures needs to be delete to not leak memory. This
adds an ExportedTextureDeleter helper class that can be used
to delete exported textures while still making sure the
compositor had a chance to consume them.

TBR=reveman@chromium.org

(cherry picked from commit 869d13439f5712193f27ded17a6300b67fd318f3)

Bug:  764007 
Test: manual
Change-Id: I18fa0494282f2352e73b320c05add8d944615213
Reviewed-on: https://chromium-review.googlesource.com/669586
Commit-Queue: David Reveman <reveman@chromium.org>
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#502518}
Reviewed-on: https://chromium-review.googlesource.com/671872
Reviewed-by: David Reveman <reveman@chromium.org>
Cr-Commit-Position: refs/branch-heads/3202@{#311}
Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098}
[modify] https://crrev.com/032a160140f7b56a4066862f4be1cfa84de22c90/ash/fast_ink/fast_ink_view.cc
[modify] https://crrev.com/032a160140f7b56a4066862f4be1cfa84de22c90/ash/fast_ink/fast_ink_view.h

Sign in to add a comment