Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[Impeller] Implement tiled texture contents for image color source#35264

Merged
auto-submit[bot] merged 5 commits intoflutter:mainfrom
ColdPaleLight:impeller_image_shader
Aug 11, 2022
Merged

[Impeller] Implement tiled texture contents for image color source#35264
auto-submit[bot] merged 5 commits intoflutter:mainfrom
ColdPaleLight:impeller_image_shader

Conversation

@ColdPaleLight
Copy link
Member

@ColdPaleLight ColdPaleLight commented Aug 9, 2022

fix flutter/flutter#109224

IYIsZx7mwu

origin_img_v2_4024470d-369d-40b8-b8ec-9001cccd6f6g

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on
    writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@ColdPaleLight ColdPaleLight marked this pull request as draft August 9, 2022 13:03
@ColdPaleLight ColdPaleLight self-assigned this Aug 9, 2022
@ColdPaleLight ColdPaleLight force-pushed the impeller_image_shader branch from 2930f21 to dff0d8d Compare August 10, 2022 14:07
@ColdPaleLight ColdPaleLight marked this pull request as ready for review August 10, 2022 14:25
void SetTexture(std::shared_ptr<Texture> texture);

void SetTileModes(Entity::TileMode horizontal_tile_mode,
Entity::TileMode vertical_tile_mode);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason we can't just add this new option to the existing TextureContents instead? We can always add more specialized shaders later when optimizing for common cases, but I don't think it would necessitate a separate command factory.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we add the new option to TextureContents, we probably also need to add a Type to TextureContents to identify whether it is a kDefault type or kTiled type,because the way they calculate texture_coords is very different:

TextureContents

    const auto tess_result = Tessellator{}.Tessellate(
        path_.GetFillType(), path_.CreatePolyline(),
        [this, &vertex_builder, &coverage_rect, &texture_size](Point vtx) {
          VS::PerVertexData data;
          data.position = vtx;
          auto coverage_coords =
              (vtx - coverage_rect->origin) / coverage_rect->size;
          data.texture_coords =
              (source_rect_.origin + source_rect_.size * coverage_coords) /
              texture_size;
          vertex_builder.AppendVertex(data);
        });

TiledTextureContents (matrix is ​​not supported yet)

    const auto tess_result =
        Tessellator{}.Tessellate(path_.GetFillType(), path_.CreatePolyline(),
                                 [&vertex_builder, &texture_size](Point vtx) {
                                   VS::PerVertexData data;
                                   data.position = vtx;
                                   data.texture_coords = vtx / texture_size;
                                   vertex_builder.AppendVertex(data);
                                 });

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting -- I see the other reason for the difference: The UVs don't get mapped to the coverage rect.

Copy link
Contributor

@bdero bdero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@ColdPaleLight ColdPaleLight changed the title [Impeller] Implement tiled texture contents for image shader [Impeller] Implement tiled texture contents for image color source Aug 11, 2022
@ColdPaleLight ColdPaleLight added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 11, 2022
@auto-submit auto-submit bot merged commit 5b1cb0c into flutter:main Aug 11, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

autosubmit Merge PR when tree becomes green via auto submit App e: impeller

Projects

No open projects
Archived in project

Development

Successfully merging this pull request may close these issues.

[Impeller] Implement tiled texture contents for image shader

2 participants