Check more optional accesses (#42528)

Related flutter/flutter#127701

Author: zanderso

flow/layers/layer_raster_cache_item.cc

@@ -7,9 +7,6 @@
 #include "flutter/flow/raster_cache_item.h"
 #include "flutter/flow/raster_cache_util.h"
 
-// TODO(zanderso): https://github.com/flutter/flutter/issues/127701
-// NOLINTBEGIN(bugprone-unchecked-optional-access)
-
 namespace flutter {
 
 LayerRasterCacheItem::LayerRasterCacheItem(Layer* layer,
@@ -143,7 +140,8 @@ static const auto* flow_type = "RasterCacheFlow::Layer";
 
 bool LayerRasterCacheItem::TryToPrepareRasterCache(const PaintContext& context,
                                                    bool parent_cached) const {
-  if (!context.raster_cache || parent_cached) {
+  auto maybe_id = GetId();
+  if (!maybe_id.has_value() || !context.raster_cache || parent_cached) {
     return false;
   }
   if (cache_state_ != kNone) {
@@ -157,8 +155,9 @@ bool LayerRasterCacheItem::TryToPrepareRasterCache(const PaintContext& context,
           .flow_type          = flow_type,
           // clang-format on
       };
+      auto id = maybe_id.value();
       return context.raster_cache->UpdateCacheEntry(
-          GetId().value(), r_context,
+          id, r_context,
           [ctx = context, cache_state = cache_state_,
            layer = layer_](DlCanvas* canvas) {
             Rasterize(cache_state, layer, ctx, canvas);
@@ -176,7 +175,7 @@ bool LayerRasterCacheItem::Draw(const PaintContext& context,
 bool LayerRasterCacheItem::Draw(const PaintContext& context,
                                 DlCanvas* canvas,
                                 const DlPaint* paint) const {
-  if (!context.raster_cache || !canvas) {
+  if (!layer_children_id_.has_value() || !context.raster_cache || !canvas) {
     return false;
   }
   switch (cache_state_) {
@@ -193,5 +192,3 @@ bool LayerRasterCacheItem::Draw(const PaintContext& context,
 }
 
 }  // namespace flutter
-
-// NOLINTEND(bugprone-unchecked-optional-access)

impeller/display_list/skia_conversions.cc

@@ -4,9 +4,6 @@
 
 #include "impeller/display_list/skia_conversions.h"
 
-// TODO(zanderso): https://github.com/flutter/flutter/issues/127701
-// NOLINTBEGIN(bugprone-unchecked-optional-access)
-
 namespace impeller {
 namespace skia_conversions {
 
@@ -24,7 +21,7 @@ std::optional<Rect> ToRect(const SkRect* rect) {
 std::vector<Rect> ToRects(const SkRect tex[], int count) {
   auto result = std::vector<Rect>();
   for (int i = 0; i < count; i++) {
-    result.push_back(ToRect(&tex[i]).value());
+    result.push_back(ToRect(tex[i]));
   }
   return result;
 }
@@ -180,5 +177,3 @@ std::optional<impeller::PixelFormat> ToPixelFormat(SkColorType type) {
 
 }  // namespace skia_conversions
 }  // namespace impeller
-
-// NOLINTEND(bugprone-unchecked-optional-access)

impeller/entity/contents/content_context.cc

@@ -16,9 +16,6 @@
 #include "impeller/renderer/render_target.h"
 #include "impeller/tessellator/tessellator.h"
 
-// TODO(zanderso): https://github.com/flutter/flutter/issues/127701
-// NOLINTBEGIN(bugprone-unchecked-optional-access)
-
 namespace impeller {
 
 void ContentContextOptions::ApplyToPipelineDescriptor(
@@ -33,13 +30,7 @@ void ContentContextOptions::ApplyToPipelineDescriptor(
   desc.SetSampleCount(sample_count);
 
   ColorAttachmentDescriptor color0 = *desc.GetColorAttachmentDescriptor(0u);
-  if (!color_attachment_pixel_format.has_value()) {
-    VALIDATION_LOG << "Color attachment pixel format must be set.";
-    color0.format = PixelFormat::kB8G8R8A8UNormInt;
-  } else {
-    color0.format = *color_attachment_pixel_format;
-  }
-  color0.format = *color_attachment_pixel_format;
+  color0.format = color_attachment_pixel_format.value_or(PixelFormat::kUnknown);
   color0.alpha_blend_op = BlendOperation::kAdd;
   color0.color_blend_op = BlendOperation::kAdd;
 
@@ -137,9 +128,9 @@ void ContentContextOptions::ApplyToPipelineDescriptor(
     desc.ClearStencilAttachments();
   }
 
-  if (desc.GetFrontStencilAttachmentDescriptor().has_value()) {
-    StencilAttachmentDescriptor stencil =
-        desc.GetFrontStencilAttachmentDescriptor().value();
+  auto maybe_stencil = desc.GetFrontStencilAttachmentDescriptor();
+  if (maybe_stencil.has_value()) {
+    StencilAttachmentDescriptor stencil = maybe_stencil.value();
     stencil.stencil_compare = stencil_compare;
     stencil.depth_stencil_pass = stencil_operation;
     desc.SetStencilAttachmentDescriptors(stencil);
@@ -312,9 +303,9 @@ ContentContext::ContentContext(std::shared_ptr<Context> context)
         context_->GetPipelineLibrary()->GetPipeline(uv_pipeline_desc).Get();
   }
 
-  if (solid_fill_pipelines_[{}]->GetDescriptor().has_value()) {
-    auto clip_pipeline_descriptor =
-        solid_fill_pipelines_[{}]->GetDescriptor().value();
+  auto maybe_pipeline_desc = solid_fill_pipelines_[{}]->GetDescriptor();
+  if (maybe_pipeline_desc.has_value()) {
+    auto clip_pipeline_descriptor = maybe_pipeline_desc.value();
     clip_pipeline_descriptor.SetLabel("Clip Pipeline");
     // Disable write to all color attachments.
     auto color_attachments =
@@ -412,5 +403,3 @@ void ContentContext::SetWireframe(bool wireframe) {
 }
 
 }  // namespace impeller
-
-// NOLINTEND(bugprone-unchecked-optional-access)

impeller/entity/contents/filters/filter_contents.cc

@@ -28,9 +28,6 @@
 #include "impeller/renderer/command_buffer.h"
 #include "impeller/renderer/render_pass.h"
 
-// TODO(zanderso): https://github.com/flutter/flutter/issues/127701
-// NOLINTBEGIN(bugprone-unchecked-optional-access)
-
 namespace impeller {
 
 std::shared_ptr<FilterContents> FilterContents::MakeDirectionalGaussianBlur(
@@ -180,8 +177,9 @@ bool FilterContents::Render(const ContentContext& renderer,
 std::optional<Rect> FilterContents::GetLocalCoverage(
     const Entity& local_entity) const {
   auto coverage = GetFilterCoverage(inputs_, local_entity, effect_transform_);
-  if (GetCoverageHint().has_value() && coverage.has_value()) {
-    coverage = coverage->Intersection(*GetCoverageHint());
+  auto coverage_hint = GetCoverageHint();
+  if (coverage_hint.has_value() && coverage.has_value()) {
+    coverage = coverage->Intersection(coverage_hint.value());
   }
 
   return coverage;
@@ -272,5 +270,3 @@ Matrix FilterContents::GetTransform(const Matrix& parent_transform) const {
 }
 
 }  // namespace impeller
-
-// NOLINTEND(bugprone-unchecked-optional-access)

impeller/entity/entity_pass.cc

@@ -34,9 +34,6 @@
 #include "impeller/entity/contents/checkerboard_contents.h"
 #endif  // IMPELLER_DEBUG
 
-// TODO(zanderso): https://github.com/flutter/flutter/issues/127701
-// NOLINTBEGIN(bugprone-unchecked-optional-access)
-
 namespace impeller {
 
 EntityPass::EntityPass() = default;
@@ -317,8 +314,9 @@ bool EntityPass::Render(ContentContext& renderer,
 
   // If a root stencil was provided by the caller, then verify that it has a
   // configuration which can be used to render this pass.
-  if (root_render_target.GetStencilAttachment().has_value()) {
-    auto stencil_texture = root_render_target.GetStencilAttachment()->texture;
+  auto stencil_attachment = root_render_target.GetStencilAttachment();
+  if (stencil_attachment.has_value()) {
+    auto stencil_texture = stencil_attachment->texture;
     if (!stencil_texture) {
       VALIDATION_LOG << "The root RenderTarget must have a stencil texture.";
       return false;
@@ -437,20 +435,23 @@ EntityPass::EntityResult EntityPass::GetEntityForElement(
       pass_context.EndPass();
     }
 
-    if (stencil_coverage_stack.empty() ||
-        !stencil_coverage_stack.back().coverage.has_value()) {
+    if (stencil_coverage_stack.empty()) {
       // The current clip is empty. This means the pass texture won't be
       // visible, so skip it.
       return EntityPass::EntityResult::Skip();
     }
+    auto stencil_coverage_back = stencil_coverage_stack.back().coverage;
+    if (!stencil_coverage_back.has_value()) {
+      return EntityPass::EntityResult::Skip();
+    }
 
     // The maximum coverage of the subpass. Subpasses textures should never
     // extend outside the parent pass texture or the current clip coverage.
     auto coverage_limit =
         Rect(global_pass_position, Size(pass_context.GetPassTarget()
                                             .GetRenderTarget()
                                             .GetRenderTargetSize()))
-            .Intersection(*stencil_coverage_stack.back().coverage);
+            .Intersection(stencil_coverage_back.value());
     if (!coverage_limit.has_value()) {
       return EntityPass::EntityResult::Skip();
     }
@@ -913,5 +914,3 @@ void EntityPass::SetEnableOffscreenCheckerboard(bool enabled) {
 }
 
 }  // namespace impeller
-
-// NOLINTEND(bugprone-unchecked-optional-access)

shell/common/shell.cc

@@ -41,9 +41,6 @@
 #include "third_party/skia/include/utils/SkBase64.h"
 #include "third_party/tonic/common/log.h"
 
-// TODO(zanderso): https://github.com/flutter/flutter/issues/127701
-// NOLINTBEGIN(bugprone-unchecked-optional-access)
-
 namespace flutter {
 
 constexpr char kSkiaChannel[] = "flutter/skia";
@@ -1556,6 +1553,8 @@ fml::TimePoint Shell::GetLatestFrameTargetTime() const {
   std::scoped_lock time_recorder_lock(time_recorder_mutex_);
   FML_CHECK(latest_frame_target_time_.has_value())
       << "GetLatestFrameTargetTime called before OnAnimatorBeginFrame";
+  // Covered by FML_CHECK().
+  // NOLINTNEXTLINE(bugprone-unchecked-optional-access)
   return latest_frame_target_time_.value();
 }
 
@@ -2147,5 +2146,3 @@ Shell::GetConcurrentWorkerTaskRunner() const {
 }
 
 }  // namespace flutter
-
-// NOLINTEND(bugprone-unchecked-optional-access)