Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CP] Add pixel snapping conditional on presence of raster cache #111231

Closed
jonahwilliams opened this issue Sep 9, 2022 · 7 comments
Closed

[CP] Add pixel snapping conditional on presence of raster cache #111231

jonahwilliams opened this issue Sep 9, 2022 · 7 comments
Labels
cp: approved Approved cherry-pick request

Comments

@jonahwilliams
Copy link
Member

jonahwilliams commented Sep 9, 2022

issue_link

#111145

Commit Hash

07ebf3c

Target

stable

pr_link

flutter/engine#36022

Impacted Users

All desktop developers

Impact Description

Application will render with fuzzy appearance on low DPR common on desktop devices.

Workaround

There is no workaround.

Risk

medium

Test Coverage

yes

Validation Steps

Verify that unit tests pass and sample applications linked from above issue render correctly

@jonahwilliams jonahwilliams added the cp: review Cherry-picks in the review queue label Sep 9, 2022
@jonahwilliams
Copy link
Member Author

jonahwilliams commented Sep 9, 2022

The following test app has been used to verify raster cached layers are appropriate. This requires a monitor with a real 1x DPR.

import 'dart:async';
import 'dart:ui' as ui;
import 'package:flutter/material.dart';

void main() {
  runApp(MaterialApp(
    home: ShaderMaskTestPage(),
  ));
}

class OpacityTestPage extends StatefulWidget {
  const OpacityTestPage({super.key});

  @override
  State<OpacityTestPage> createState() => _OpacityTestPageState();
}

class _OpacityTestPageState extends State<OpacityTestPage> {
  bool _opaque = true;

  @override
  void initState() {
    super.initState();
    Timer.periodic(const Duration(seconds: 1), (timer) {
      setState(() => _opaque = !_opaque);
    });
  }

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      backgroundColor: Colors.red,
      body: Padding(
        padding: const EdgeInsets.all(55.5),
        child: Opacity(
          opacity: _opaque ? 1 : 0.95,
          child: Container(
            color: Colors.white,
            child: const Center(
              child: Text(
                'Flutter',
                style: TextStyle(fontSize: 256, color: Colors.blue),
              ),
            ),
          ),
        ),
      ),
    );
  }
}

class ClipTestPage extends StatefulWidget {
  const ClipTestPage({super.key});

  @override
  State<ClipTestPage> createState() => _ClipTestPageState();
}

class _ClipTestPageState extends State<ClipTestPage> {
  bool _opaque = true;

  @override
  void initState() {
    super.initState();
    Timer.periodic(const Duration(seconds: 1), (timer) {
      setState(() => _opaque = !_opaque);
    });
  }

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      backgroundColor: Colors.red,
      body: Padding(
        padding: const EdgeInsets.all(55.5),
        child: ClipRect(
          clipBehavior: _opaque ? Clip.antiAliasWithSaveLayer : Clip.hardEdge,
          child: Container(
            color: Colors.white,
            child: const Center(
              child: Text(
                'Flutter',
                style: TextStyle(fontSize: 256, color: Colors.blue),
              ),
            ),
          ),
        ),
      ),
    );
  }
}

class ColorFilterTestPage extends StatefulWidget {
  const ColorFilterTestPage({super.key});

  @override
  State<ColorFilterTestPage> createState() => ColorFilterTestPageState();
}

class ColorFilterTestPageState extends State<ColorFilterTestPage> {
  bool _opaque = true;

  @override
  void initState() {
    super.initState();
    Timer.periodic(const Duration(seconds: 1), (timer) {
      setState(() => _opaque = !_opaque);
    });
  }

  @override
  Widget build(BuildContext context) {
    Widget child = Container(
      color: Colors.white,
      child: const Center(
        child: Text(
          'Flutter',
          style: TextStyle(fontSize: 256, color: Colors.blue),
        ),
      ),
    );
    if (_opaque) {
      child = ColorFiltered(
        colorFilter: ColorFilter.mode(Colors.blue, BlendMode.darken),
        child: child,
      );
    }
    return Scaffold(
      backgroundColor: Colors.red,
      body: Padding(
        padding: const EdgeInsets.all(55.5),
        child: child,
      ),
    );
  }
}


class ImageFilterTestPage extends StatefulWidget {
  const ImageFilterTestPage({super.key});

  @override
  State<ImageFilterTestPage> createState() => ImageFilterTestPageState();
}

class ImageFilterTestPageState extends State<ImageFilterTestPage> {
  bool _opaque = true;

  @override
  void initState() {
    super.initState();
    Timer.periodic(const Duration(seconds: 1), (timer) {
      setState(() => _opaque = !_opaque);
    });
  }

  @override
  Widget build(BuildContext context) {
    Widget child = Container(
      color: Colors.white,
      child: const Center(
        child: Text(
          'Flutter',
          style: TextStyle(fontSize: 256, color: Colors.blue),
        ),
      ),
    );
    if (_opaque) {
      child = ImageFiltered(
        imageFilter: ui.ImageFilter.blur(sigmaX: 2, sigmaY: 2),
        child: child,
      );
    }
    return Scaffold(
      backgroundColor: Colors.red,
      body: Padding(
        padding: const EdgeInsets.all(55.5),
        child: child,
      ),
    );
  }
}


class ShaderMaskTestPage extends StatefulWidget {
  const ShaderMaskTestPage({super.key});

  @override
  State<ShaderMaskTestPage> createState() => ShaderMaskTestPageState();
}

class ShaderMaskTestPageState extends State<ShaderMaskTestPage> {
  bool _opaque = true;

  @override
  void initState() {
    super.initState();
    Timer.periodic(const Duration(seconds: 1), (timer) {
      setState(() => _opaque = !_opaque);
    });
  }

  @override
  Widget build(BuildContext context) {
    Widget child = Container(
      color: Colors.white,
      child: const Center(
        child: Text(
          'Flutter',
          style: TextStyle(fontSize: 256, color: Colors.blue),
        ),
      ),
    );
    if (_opaque) {
      child = ShaderMask(
        shaderCallback: LinearGradient(colors: [Colors.red, Colors.blue]).createShader,
        child: child,
      );
    }
    return Scaffold(
      backgroundColor: Colors.red,
      body: Padding(
        padding: const EdgeInsets.all(55.5),
        child: child,
      ),
    );
  }
}

@itsjustkevin
Copy link
Contributor

itsjustkevin commented Sep 9, 2022

@flar and @zanderso thoughts on this cherry-pick?

@zanderso
Copy link
Member

zanderso commented Sep 9, 2022

I agree in principle that this should be cherry-picked, but I don't have enough familiarity to review the PR. I'll request a review from @flar there.

@itsjustkevin
Copy link
Contributor

itsjustkevin commented Sep 12, 2022

@zanderso and @jonahwilliams do we have someone that has context around this outside of @flar?

@zanderso
Copy link
Member

zanderso commented Sep 12, 2022

@dnfield has reviewed.

@itsjustkevin itsjustkevin added cp: approved Approved cherry-pick request and removed cp: review Cherry-picks in the review queue labels Sep 12, 2022
@itsjustkevin
Copy link
Contributor

itsjustkevin commented Sep 14, 2022

Merged in Flutter 3.3.2.

@github-actions
Copy link

github-actions bot commented Sep 28, 2022

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cp: approved Approved cherry-pick request
Projects
None yet
Development

No branches or pull requests

3 participants