Bug 220937 - [GPU Process] Move encoding and decoding Gradient and Pattern to WebKit
Summary: [GPU Process] Move encoding and decoding Gradient and Pattern to WebKit
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Said Abou-Hallawa
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-01-25 11:17 PST by Said Abou-Hallawa
Modified: 2022-02-10 16:49 PST (History)
6 users (show)

See Also:


Attachments
Patch (31.21 KB, patch)
2021-01-25 11:23 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Said Abou-Hallawa 2021-01-25 11:17:01 PST
This will simplify the header files Gradient.h and Pattern.h. This will also make encoding and decoding these objects consistent with NativeImage and Font.
Comment 1 Said Abou-Hallawa 2021-01-25 11:23:37 PST
Created attachment 418314 [details]
Patch
Comment 2 Tim Horton 2021-01-25 11:31:42 PST
This seems like a step in the wrong direction, no? For other GPUProcess things (DisplayListItems etc.) and other modern types we've been keeping the encoders adjacent to the types they encode.
Comment 3 Said Abou-Hallawa 2021-01-25 11:44:30 PST
The plan is to send Gradient and Pattern from WebProcess to GPUProcess as IPC::GradientReference and IPC::PatternReference via sendMessage().

I am not sure how this can be implemented if we keep the encoding and decoding as methods of Gradient and Pattern.
Comment 4 Sam Weinig 2021-01-25 12:49:40 PST
(In reply to Said Abou-Hallawa from comment #3)
> The plan is to send Gradient and Pattern from WebProcess to GPUProcess as
> IPC::GradientReference and IPC::PatternReference via sendMessage().
> 
> I am not sure how this can be implemented if we keep the encoding and
> decoding as methods of Gradient and Pattern.

What are GradientReferences and PatternReferences and how do they work?
Comment 5 Said Abou-Hallawa 2021-01-25 12:55:49 PST
(In reply to Sam Weinig from comment #4)
> (In reply to Said Abou-Hallawa from comment #3)
> > The plan is to send Gradient and Pattern from WebProcess to GPUProcess as
> > IPC::GradientReference and IPC::PatternReference via sendMessage().
> > 
> > I am not sure how this can be implemented if we keep the encoding and
> > decoding as methods of Gradient and Pattern.
> 
> What are GradientReferences and PatternReferences and how do they work?

Because of limitation/bugs in messages.py we can't process templates in the messages.in files. For example to send an IPC messages with Ref<Font> as an argument we have to make this definition in FontReference.h:

    using FontReference = Ref<WebCore::Font>;

Then we can use it in RemoteRenderingBackend.messages.in:

    CacheFont(IPC::FontReference font)

GradientReferences and PatternReferences will be the same as FontReference.
Comment 6 Sam Weinig 2021-01-26 14:21:58 PST
(In reply to Said Abou-Hallawa from comment #5)
> (In reply to Sam Weinig from comment #4)
> > (In reply to Said Abou-Hallawa from comment #3)
> > > The plan is to send Gradient and Pattern from WebProcess to GPUProcess as
> > > IPC::GradientReference and IPC::PatternReference via sendMessage().
> > > 
> > > I am not sure how this can be implemented if we keep the encoding and
> > > decoding as methods of Gradient and Pattern.
> > 
> > What are GradientReferences and PatternReferences and how do they work?
> 
> Because of limitation/bugs in messages.py we can't process templates in the
> messages.in files. For example to send an IPC messages with Ref<Font> as an
> argument we have to make this definition in FontReference.h:
> 
>     using FontReference = Ref<WebCore::Font>;
> 
> Then we can use it in RemoteRenderingBackend.messages.in:
> 
>     CacheFont(IPC::FontReference font)
> 
> GradientReferences and PatternReferences will be the same as FontReference.

Sorry, I wasn't clear in my question. What it does mean to have a GradientReference? Will you be uniquing gradients, sending over the data once, and then sending an identifier? Since this code you are referring to does not exist yet, you need to be clear about what exactly you are describing.
Comment 7 Said Abou-Hallawa 2021-01-29 08:57:44 PST
(In reply to Sam Weinig from comment #6)
> (In reply to Said Abou-Hallawa from comment #5)
> > (In reply to Sam Weinig from comment #4)
> > > (In reply to Said Abou-Hallawa from comment #3)
> > > > The plan is to send Gradient and Pattern from WebProcess to GPUProcess as
> > > > IPC::GradientReference and IPC::PatternReference via sendMessage().
> > > > 
> > > > I am not sure how this can be implemented if we keep the encoding and
> > > > decoding as methods of Gradient and Pattern.
> > > 
> > > What are GradientReferences and PatternReferences and how do they work?
> > 
> > Because of limitation/bugs in messages.py we can't process templates in the
> > messages.in files. For example to send an IPC messages with Ref<Font> as an
> > argument we have to make this definition in FontReference.h:
> > 
> >     using FontReference = Ref<WebCore::Font>;
> > 
> > Then we can use it in RemoteRenderingBackend.messages.in:
> > 
> >     CacheFont(IPC::FontReference font)
> > 
> > GradientReferences and PatternReferences will be the same as FontReference.
> 
> Sorry, I wasn't clear in my question. What it does mean to have a
> GradientReference? Will you be uniquing gradients, sending over the data
> once, and then sending an identifier? Since this code you are referring to
> does not exist yet, you need to be clear about what exactly you are
> describing.

Yes your expectation is correct: "uniquing gradients, sending over the data
once, and then sending an identifier".

Ideally we do not want to create gradients or patterns in WebP but because DOM rendering can still happen in WebP, we have to have two copies of the gradient: one is in WebP and the second is in GPUP.

For the GPUP gradient and patterns, SetState::encode() and SetState::decode() will deal with the identifiers only. Replayer::applyItem() will resolve the identifiers to the actual cached objects. Please see the implementation of these functions in https://bugs.webkit.org/attachment.cgi?id=416597&action=review.
Comment 8 Sam Weinig 2021-01-29 11:09:11 PST
(In reply to Said Abou-Hallawa from comment #7)
> (In reply to Sam Weinig from comment #6)
> > (In reply to Said Abou-Hallawa from comment #5)
> > > (In reply to Sam Weinig from comment #4)
> > > > (In reply to Said Abou-Hallawa from comment #3)
> > > > > The plan is to send Gradient and Pattern from WebProcess to GPUProcess as
> > > > > IPC::GradientReference and IPC::PatternReference via sendMessage().
> > > > > 
> > > > > I am not sure how this can be implemented if we keep the encoding and
> > > > > decoding as methods of Gradient and Pattern.
> > > > 
> > > > What are GradientReferences and PatternReferences and how do they work?
> > > 
> > > Because of limitation/bugs in messages.py we can't process templates in the
> > > messages.in files. For example to send an IPC messages with Ref<Font> as an
> > > argument we have to make this definition in FontReference.h:
> > > 
> > >     using FontReference = Ref<WebCore::Font>;
> > > 
> > > Then we can use it in RemoteRenderingBackend.messages.in:
> > > 
> > >     CacheFont(IPC::FontReference font)
> > > 
> > > GradientReferences and PatternReferences will be the same as FontReference.
> > 
> > Sorry, I wasn't clear in my question. What it does mean to have a
> > GradientReference? Will you be uniquing gradients, sending over the data
> > once, and then sending an identifier? Since this code you are referring to
> > does not exist yet, you need to be clear about what exactly you are
> > describing.
> 
> Yes your expectation is correct: "uniquing gradients, sending over the data
> once, and then sending an identifier".

Ok, in that case, I think keeping the default encoding/decoding in WebCore would be the best way to go.

Then, where ever does the uniquing (be it in WebCore or WebKit) can use that encoder for the initial encode / decode.
Comment 9 Radar WebKit Bug Importer 2021-02-01 11:17:13 PST
<rdar://problem/73837500>
Comment 10 Said Abou-Hallawa 2021-02-03 15:42:59 PST
(In reply to Sam Weinig from comment #8)
> (In reply to Said Abou-Hallawa from comment #7)
> > (In reply to Sam Weinig from comment #6)
> > > (In reply to Said Abou-Hallawa from comment #5)
> > > > (In reply to Sam Weinig from comment #4)
> > > > > (In reply to Said Abou-Hallawa from comment #3)
> > > > > > The plan is to send Gradient and Pattern from WebProcess to GPUProcess as
> > > > > > IPC::GradientReference and IPC::PatternReference via sendMessage().
> > > > > > 
> > > > > > I am not sure how this can be implemented if we keep the encoding and
> > > > > > decoding as methods of Gradient and Pattern.
> > > > > 
> > > > > What are GradientReferences and PatternReferences and how do they work?
> > > > 
> > > > Because of limitation/bugs in messages.py we can't process templates in the
> > > > messages.in files. For example to send an IPC messages with Ref<Font> as an
> > > > argument we have to make this definition in FontReference.h:
> > > > 
> > > >     using FontReference = Ref<WebCore::Font>;
> > > > 
> > > > Then we can use it in RemoteRenderingBackend.messages.in:
> > > > 
> > > >     CacheFont(IPC::FontReference font)
> > > > 
> > > > GradientReferences and PatternReferences will be the same as FontReference.
> > > 
> > > Sorry, I wasn't clear in my question. What it does mean to have a
> > > GradientReference? Will you be uniquing gradients, sending over the data
> > > once, and then sending an identifier? Since this code you are referring to
> > > does not exist yet, you need to be clear about what exactly you are
> > > describing.
> > 
> > Yes your expectation is correct: "uniquing gradients, sending over the data
> > once, and then sending an identifier".
> 
> Ok, in that case, I think keeping the default encoding/decoding in WebCore
> would be the best way to go.
> 
> Then, where ever does the uniquing (be it in WebCore or WebKit) can use that
> encoder for the initial encode / decode.

This is the code of encoding the pattern in WebCore:

void Pattern::encode(Encoder& encoder) const
{
    ImageHandle imageHandle;
    imageHandle.image = m_tileImage.ptr();
    encoder << imageHandle;
    encoder << m_patternSpaceTransformation;
    encoder << m_repeatX;
    encoder << m_repeatY;
}

But the image is encoded by ArgumentCoder<ImageHandle>::encode() which is in WebKit.