Bug 76934 - [chromium] Dirty pre-painted tiles should be deleted on impl side
Summary: [chromium] Dirty pre-painted tiles should be deleted on impl side
Status: RESOLVED WORKSFORME
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-01-24 12:59 PST by Dana Jansens
Modified: 2012-03-02 11:43 PST (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Dana Jansens 2012-01-24 12:59:28 PST
Drawing pre-painted tiles that don't match the latest commit is going to cause bad visual artifacts. (e.g. video tearing at the tile boundary.)

It is possible that this is done already (please say so).  But if not it is a placeholder for me to come back to soon.
Comment 1 Adrienne Walker 2012-01-24 13:08:49 PST
I don't think this is needed.

During every commit, we toss every tile on the impl side and then only sync the valid and non-dirty ones over.  Take a look at TiledLayerChromium::pushPropertiesTo.
Comment 2 Adrienne Walker 2012-01-24 13:10:11 PST
(In reply to comment #1)
> I don't think this is needed.
> 
> During every commit, we toss every tile on the impl side and then only sync the valid and non-dirty ones over.  Take a look at TiledLayerChromium::pushPropertiesTo.

Also, if anything, we're too conservative.  If a tile gets invalidated during paint, we won't sync it over even though its contents are technically valid for that frame.
Comment 3 Dana Jansens 2012-01-24 13:22:47 PST
(In reply to comment #1)
> I don't think this is needed.
> 
> During every commit, we toss every tile on the impl side and then only sync the valid and non-dirty ones over.  Take a look at TiledLayerChromium::pushPropertiesTo.

I am freshly confused what the purpose of layerTreeHost()->deleteTextureAfterCommit() is then. If we just allocated a new texture on paint side, the impl side should be deleted anyways?
Comment 4 Adrienne Walker 2012-01-24 14:12:08 PST
(In reply to comment #3)
> (In reply to comment #1)
> > I don't think this is needed.
> > 
> > During every commit, we toss every tile on the impl side and then only sync the valid and non-dirty ones over.  Take a look at TiledLayerChromium::pushPropertiesTo.
> 
> I am freshly confused what the purpose of layerTreeHost()->deleteTextureAfterCommit() is then. If we just allocated a new texture on paint side, the impl side should be deleted anyways?

It's to support interleaved uploads.  If a texture is currently in use on the impl side, you can't reuse that texture id if you're going to interleave drawing while you upload and before the commit is done.  So, you allocate a new id and mark the old one as no longer needed once the commit completes.
Comment 5 David Reveman 2012-01-25 08:10:35 PST
(In reply to comment #1)
> I don't think this is needed.
> 
> During every commit, we toss every tile on the impl side and then only sync the valid and non-dirty ones over.  Take a look at TiledLayerChromium::pushPropertiesTo.

It shouldn't be possible to get artifacts from the current system. Dana and I talked about this yesterday. This isn't really a problem with the current system, it's just an observation:

Dirty tile textures are not actually deleted (as in context3D->deleteTexture being called).

That's part of the design and the texture manager's ability to reuse textures. Would we want some exceptions to this now that we we have pre-painting? I guess it could possibly be worth not deleting dirty non-visible tile textures but deleting dirty non-visible pre-painted tile textures. Basically having a different memory limit for pre-painted tiles. I'm not convinced but figured raising the question wouldn't hurt.
Comment 6 Dana Jansens 2012-03-02 11:43:25 PST
I get how this works now