Changeset 229316 in webkit


Ignore:
Timestamp:
Mar 6, 2018 3:35:22 AM (6 years ago)
Author:
zandobersek@gmail.com
Message:

[CoordGraphics] Clean up CoordinatedImageBacking
​https://bugs.webkit.org/show_bug.cgi?id=183332

Reviewed by Carlos Garcia Campos.

Source/WebCore:

Clean up the CoordinatedImageBacking class. Prefer reference values in
class functions, methods and member variables, where possible. Move
member variables into a more sensible order. Initialize a few member
variables at the place of declaration.

Drop releaseSurfaceIfNeeded() and updateVisibilityIfNeeded() methods,
integrating them into the update() method, which was the only place
where they were called from.

We don't have to keep a reference to the buffer object, since we're
not using it internally after it's been passed to the client's
updateImageBacking() implementation.

No new tests -- no change in behavior.

  • platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp:

(WebCore::CoordinatedGraphicsLayer::syncImageBacking):
(WebCore::CoordinatedGraphicsLayer::releaseImageBackingIfNeeded):

  • platform/graphics/texmap/coordinated/CoordinatedImageBacking.cpp:

(WebCore::CoordinatedImageBacking::getCoordinatedImageBackingID):
(WebCore::CoordinatedImageBacking::CoordinatedImageBacking):
(WebCore::CoordinatedImageBacking::addHost):
(WebCore::CoordinatedImageBacking::removeHost):
(WebCore::CoordinatedImageBacking::update):
(WebCore::CoordinatedImageBacking::clearContentsTimerFired):
(WebCore::CoordinatedImageBacking::create): Deleted.
(WebCore::CoordinatedImageBacking::markDirty): Deleted.
(WebCore::CoordinatedImageBacking::releaseSurfaceIfNeeded): Deleted.
(WebCore::CoordinatedImageBacking::updateVisibilityIfNeeded): Deleted.

  • platform/graphics/texmap/coordinated/CoordinatedImageBacking.h:

Source/WebKit:

  • WebProcess/WebPage/CoordinatedGraphics/CompositingCoordinator.cpp:

(WebKit::CompositingCoordinator::createImageBackingIfNeeded):
Adjust call to CoordinatedImageBacking::getCoordinatedImageBackingID().

Location:
trunk/Source
Files:
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r229313 r229316  
     12018-03-06  Zan Dobersek  <zdobersek@igalia.com>
     2
     3        [CoordGraphics] Clean up CoordinatedImageBacking
     4        https://bugs.webkit.org/show_bug.cgi?id=183332
     5
     6        Reviewed by Carlos Garcia Campos.
     7
     8        Clean up the CoordinatedImageBacking class. Prefer reference values in
     9        class functions, methods and member variables, where possible. Move
     10        member variables into a more sensible order. Initialize a few member
     11        variables at the place of declaration.
     12
     13        Drop releaseSurfaceIfNeeded() and updateVisibilityIfNeeded() methods,
     14        integrating them into the update() method, which was the only place
     15        where they were called from.
     16
     17        We don't have to keep a reference to the buffer object, since we're
     18        not using it internally after it's been passed to the client's
     19        updateImageBacking() implementation.
     20
     21        No new tests -- no change in behavior.
     22
     23        * platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp:
     24        (WebCore::CoordinatedGraphicsLayer::syncImageBacking):
     25        (WebCore::CoordinatedGraphicsLayer::releaseImageBackingIfNeeded):
     26        * platform/graphics/texmap/coordinated/CoordinatedImageBacking.cpp:
     27        (WebCore::CoordinatedImageBacking::getCoordinatedImageBackingID):
     28        (WebCore::CoordinatedImageBacking::CoordinatedImageBacking):
     29        (WebCore::CoordinatedImageBacking::addHost):
     30        (WebCore::CoordinatedImageBacking::removeHost):
     31        (WebCore::CoordinatedImageBacking::update):
     32        (WebCore::CoordinatedImageBacking::clearContentsTimerFired):
     33        (WebCore::CoordinatedImageBacking::create): Deleted.
     34        (WebCore::CoordinatedImageBacking::markDirty): Deleted.
     35        (WebCore::CoordinatedImageBacking::releaseSurfaceIfNeeded): Deleted.
     36        (WebCore::CoordinatedImageBacking::updateVisibilityIfNeeded): Deleted.
     37        * platform/graphics/texmap/coordinated/CoordinatedImageBacking.h:
     38
    1392018-03-06  Zan Dobersek  <zdobersek@igalia.com>
    240

  • trunk/Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp

    r229209 r229316  
    629629        ASSERT(m_compositedImage);
    630630
    631         bool imageInstanceReplaced = m_coordinatedImageBacking && (m_coordinatedImageBacking->id() != CoordinatedImageBacking::getCoordinatedImageBackingID(m_compositedImage.get()));
     631        bool imageInstanceReplaced = m_coordinatedImageBacking && (m_coordinatedImageBacking->id() != CoordinatedImageBacking::getCoordinatedImageBackingID(*m_compositedImage));
    632632        if (imageInstanceReplaced)
    633633            releaseImageBackingIfNeeded();
    … …  
    635635        if (!m_coordinatedImageBacking) {
    636636            m_coordinatedImageBacking = m_coordinator->createImageBackingIfNeeded(*m_compositedImage);
    637             m_coordinatedImageBacking->addHost(this);
     637            m_coordinatedImageBacking->addHost(*this);
    638638            m_layerState.imageID = m_coordinatedImageBacking->id();
    639639        }
    … …  
    791791
    792792    ASSERT(m_coordinator);
    793     m_coordinatedImageBacking->removeHost(this);
     793    m_coordinatedImageBacking->removeHost(*this);
    794794    m_coordinatedImageBacking = nullptr;
    795795    m_layerState.imageID = InvalidCoordinatedImageBackingID;

  • trunk/Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedImageBacking.cpp

    r225573 r229316  
    3636namespace WebCore {
    3737
    38 CoordinatedImageBackingID CoordinatedImageBacking::getCoordinatedImageBackingID(Image* image)
     38CoordinatedImageBackingID CoordinatedImageBacking::getCoordinatedImageBackingID(Image& image)
    3939{
    4040    // CoordinatedImageBacking keeps a RefPtr<Image> member, so the same Image pointer can not refer two different instances until CoordinatedImageBacking releases the member.
    41     return reinterpret_cast<CoordinatedImageBackingID>(image);
    42 }
    43 
    44 Ref<CoordinatedImageBacking> CoordinatedImageBacking::create(Client& client, Ref<Image>&& image)
    45 {
    46     return adoptRef(*new CoordinatedImageBacking(client, WTFMove(image)));
     41    return reinterpret_cast<CoordinatedImageBackingID>(&image);
    4742}
    4843
    4944CoordinatedImageBacking::CoordinatedImageBacking(Client& client, Ref<Image>&& image)
    50     : m_client(&client)
     45    : m_client(client)
     46    , m_id(getCoordinatedImageBackingID(image))
    5147    , m_image(WTFMove(image))
    52     , m_id(getCoordinatedImageBackingID(m_image.get()))
    5348    , m_clearContentsTimer(*this, &CoordinatedImageBacking::clearContentsTimerFired)
    54     , m_isDirty(false)
    55     , m_isVisible(false)
    5649{
    57     // FIXME: We would need to decode a small image directly into a GraphicsSurface.
    58     // http://webkit.org/b/101426
    59 
    60     m_client->createImageBacking(id());
     50    m_client.createImageBacking(m_id);
    6151}
    6252
    6353CoordinatedImageBacking::~CoordinatedImageBacking() = default;
    6454
    65 void CoordinatedImageBacking::addHost(Host* host)
     55void CoordinatedImageBacking::addHost(Host& host)
    6656{
    67     ASSERT(!m_hosts.contains(host));
    68     m_hosts.append(host);
     57    ASSERT(!m_hosts.contains(&host));
     58    m_hosts.add(&host);
    6959}
    7060
    71 void CoordinatedImageBacking::removeHost(Host* host)
     61void CoordinatedImageBacking::removeHost(Host& host)
    7262{
    73     size_t position = m_hosts.find(host);
    74     ASSERT(position != notFound);
    75     m_hosts.remove(position);
     63    m_hosts.remove(&host);
    7664
    7765    if (m_hosts.isEmpty())
    78         m_client->removeImageBacking(id());
     66        m_client.removeImageBacking(m_id);
    7967}
    8068
    81 void CoordinatedImageBacking::markDirty()
    82 {
    83     m_isDirty = true;
    84 }
     69static const Seconds clearContentsTimerInterval { 3_s };
    8570
    8671void CoordinatedImageBacking::update()
    8772{
    88     releaseSurfaceIfNeeded();
     73    bool previousIsVisible = m_isVisible;
     74    m_isVisible = std::any_of(m_hosts.begin(), m_hosts.end(),
     75        [](auto* host)
     76        {
     77            return host->imageBackingVisible();
     78        });
    8979
    90     bool changedToVisible;
    91     updateVisibilityIfNeeded(changedToVisible);
    92     if (!m_isVisible)
     80    if (!m_isVisible) {
     81        if (previousIsVisible) {
     82            ASSERT(!m_clearContentsTimer.isActive());
     83            m_clearContentsTimer.startOneShot(clearContentsTimerInterval);
     84        }
    9385        return;
     86    }
    9487
     88    bool changedToVisible = !previousIsVisible;
     89    if (m_clearContentsTimer.isActive()) {
     90        m_clearContentsTimer.stop();
     91        // We don't want to update the texture if we didn't remove the texture.
     92        changedToVisible = false;
     93    }
     94
     95    auto nativeImagePtr = m_image->nativeImageForCurrentFrame();
    9596    if (!changedToVisible) {
    9697        if (!m_isDirty)
    9798            return;
    9899
    99         if (m_nativeImagePtr == m_image->nativeImageForCurrentFrame()) {
     100        if (m_nativeImagePtr == nativeImagePtr) {
    100101            m_isDirty = false;
    101102            return;
    … …  
    103104    }
    104105
    105     m_buffer = Nicosia::Buffer::create(IntSize(m_image->size()), !m_image->currentFrameKnownToBeOpaque() ? Nicosia::Buffer::SupportsAlpha : Nicosia::Buffer::NoFlags);
    106     ASSERT(m_buffer);
     106    m_nativeImagePtr = WTFMove(nativeImagePtr);
    107107
    108     Nicosia::PaintingContext::paint(*m_buffer,
     108    auto buffer = Nicosia::Buffer::create(IntSize(m_image->size()), !m_image->currentFrameKnownToBeOpaque() ? Nicosia::Buffer::SupportsAlpha : Nicosia::Buffer::NoFlags);
     109    Nicosia::PaintingContext::paint(buffer,
    109110        [this](GraphicsContext& context)
    110111        {
    … …  
    112113            context.save();
    113114            context.clip(rect);
    114             context.drawImage(*m_image, rect, rect);
     115            context.drawImage(m_image, rect, rect);
    115116            context.restore();
    116117        });
    117118
    118     m_nativeImagePtr = m_image->nativeImageForCurrentFrame();
    119 
    120     m_client->updateImageBacking(id(), m_buffer.copyRef());
     119    m_client.updateImageBacking(m_id, WTFMove(buffer));
    121120    m_isDirty = false;
    122 }
    123 
    124 void CoordinatedImageBacking::releaseSurfaceIfNeeded()
    125 {
    126     m_buffer = nullptr;
    127 }
    128 
    129 static const Seconds clearContentsTimerInterval { 3_s };
    130 
    131 void CoordinatedImageBacking::updateVisibilityIfNeeded(bool& changedToVisible)
    132 {
    133     bool previousIsVisible = m_isVisible;
    134 
    135     m_isVisible = false;
    136     for (auto& host : m_hosts) {
    137         if (host->imageBackingVisible()) {
    138             m_isVisible = true;
    139             break;
    140         }
    141     }
    142 
    143     bool changedToInvisible = previousIsVisible && !m_isVisible;
    144     if (changedToInvisible) {
    145         ASSERT(!m_clearContentsTimer.isActive());
    146         m_clearContentsTimer.startOneShot(clearContentsTimerInterval);
    147     }
    148 
    149     changedToVisible = !previousIsVisible && m_isVisible;
    150 
    151     if (m_isVisible && m_clearContentsTimer.isActive()) {
    152         m_clearContentsTimer.stop();
    153         // We don't want to update the texture if we didn't remove the texture.
    154         changedToVisible = false;
    155     }
    156121}
    157122
    158123void CoordinatedImageBacking::clearContentsTimerFired()
    159124{
    160     m_client->clearImageBackingContents(id());
     125    m_client.clearImageBackingContents(m_id);
    161126}
    162127
    163128} // namespace WebCore
     129
    164130#endif

  • trunk/Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedImageBacking.h

    r226657 r229316  
    2424 */
    2525
    26 
    27 #ifndef CoordinatedImageBacking_h
    28 #define CoordinatedImageBacking_h
     26#pragma once
    2927
    3028#if USE(COORDINATED_GRAPHICS)
    … …  
    5755    };
    5856
    59     static Ref<CoordinatedImageBacking> create(Client&, Ref<Image>&&);
     57    static Ref<CoordinatedImageBacking> create(Client& client, Ref<Image>&& image)
     58    {
     59        return adoptRef(*new CoordinatedImageBacking(client, WTFMove(image)));
     60    }
    6061    virtual ~CoordinatedImageBacking();
    6162
    62     static CoordinatedImageBackingID getCoordinatedImageBackingID(Image*);
     63    static CoordinatedImageBackingID getCoordinatedImageBackingID(Image&);
    6364    CoordinatedImageBackingID id() const { return m_id; }
    6465
    65     void addHost(Host*);
    66     void removeHost(Host*);
     66    void addHost(Host&);
     67    void removeHost(Host&);
    6768
    6869    // When a new image is updated or an animated gif is progressed, CoordinatedGraphicsLayer calls markDirty().
    69     void markDirty();
     70    void markDirty() { m_isDirty = true; }
    7071
    7172    // Create, remove or update its backing.
    … …  
    7576    CoordinatedImageBacking(Client&, Ref<Image>&&);
    7677
    77     void releaseSurfaceIfNeeded();
    78     void updateVisibilityIfNeeded(bool& changedToVisible);
    7978    void clearContentsTimerFired();
    8079
    81     Client* m_client;
    82     RefPtr<Image> m_image;
     80    Client& m_client;
     81    HashSet<Host*> m_hosts;
     82
     83    CoordinatedImageBackingID m_id;
     84    Ref<Image> m_image;
    8385    NativeImagePtr m_nativeImagePtr;
    84     CoordinatedImageBackingID m_id;
    85     Vector<Host*> m_hosts;
    86 
    87     RefPtr<Nicosia::Buffer> m_buffer;
    8886
    8987    Timer m_clearContentsTimer;
    9088
    91     bool m_isDirty;
    92     bool m_isVisible;
    93 
     89    bool m_isDirty { false };
     90    bool m_isVisible { false };
    9491};
    9592
    9693} // namespace WebCore
     94
    9795#endif // USE(COORDINATED_GRAPHICS)
    98 
    99 #endif // CoordinatedImageBacking_h

  • trunk/Source/WebKit/ChangeLog

    r229315 r229316  
     12018-03-06  Zan Dobersek  <zdobersek@igalia.com>
     2
     3        [CoordGraphics] Clean up CoordinatedImageBacking
     4        https://bugs.webkit.org/show_bug.cgi?id=183332
     5
     6        Reviewed by Carlos Garcia Campos.
     7
     8        * WebProcess/WebPage/CoordinatedGraphics/CompositingCoordinator.cpp:
     9        (WebKit::CompositingCoordinator::createImageBackingIfNeeded):
     10        Adjust call to CoordinatedImageBacking::getCoordinatedImageBackingID().
     11
    1122018-03-06  Zan Dobersek  <zdobersek@igalia.com>
    213

  • trunk/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/CompositingCoordinator.cpp

    r228373 r229316  
    214214Ref<CoordinatedImageBacking> CompositingCoordinator::createImageBackingIfNeeded(Image& image)
    215215{
    216     CoordinatedImageBackingID imageID = CoordinatedImageBacking::getCoordinatedImageBackingID(&image);
     216    CoordinatedImageBackingID imageID = CoordinatedImageBacking::getCoordinatedImageBackingID(image);
    217217    auto addResult = m_imageBackings.ensure(imageID, [this, &image] {
    218218        return CoordinatedImageBacking::create(*this, image);
Note: See TracChangeset for help on using the changeset viewer.