[flutter_tools] Watch transitive #include headers for FragmentProgram hot reload#187945
[flutter_tools] Watch transitive #include headers for FragmentProgram hot reload#187945bkonyi wants to merge 5 commits into
Conversation
… hot reload Pass --depfile to impellerc and track dependencies in DevelopmentShaderCompiler to trigger hot reload when transitive includes are modified. Fixes flutter#186343
There was a problem hiding this comment.
Code Review
This pull request introduces tracking of transitive shader dependencies in DevelopmentShaderCompiler to determine if shaders need recompilation during DevFS syncs. It parses depfiles generated by impellerc and exposes areDependenciesModified to check for modified dependencies. Feedback highlights a potential FileSystemException when calling dep.statSync() in areDependenciesModified, which could crash the sync process and should be wrapped in a try-catch block.
| for (final File dep in deps) { | ||
| if (!dep.existsSync()) { | ||
| return true; | ||
| } | ||
| if (dep.statSync().modified.isAfter(lastCompiled)) { | ||
| return true; | ||
| } | ||
| } |
There was a problem hiding this comment.
Calling dep.statSync() can throw a FileSystemException if the file is inaccessible, has permission issues, or is deleted between the existsSync() check and the statSync() call. Since this method is called during the DevFS sync process, an unhandled exception here will crash the sync and fail the hot reload.
We should wrap the check in a try-catch block to handle FileSystemException gracefully, log it as a trace, and safely assume the dependency has been modified to trigger a recompile.
for (final File dep in deps) {
try {
if (!dep.existsSync()) {
return true;
}
if (dep.statSync().modified.isAfter(lastCompiled)) {
return true;
}
} on FileSystemException catch (e) {
_logger.printTrace('Error checking shader dependency modification time for ${dep.path}: $e');
return true;
}
}|
An existing Git SHA, To re-trigger presubmits after closing or re-opeing a PR, or pushing a HEAD commit (i.e. with |
Wrap dependency existence and stat checks in a try-catch block in areDependenciesModified to prevent crashing the DevFS sync process if a FileSystemException occurs.
|
autosubmit label was removed for flutter/flutter/187945, because The base commit of the PR is older than 7 days and can not be merged. Please merge the latest changes from the main into this branch and resubmit the PR. |
|
auto label is removed for flutter/flutter/187945, Failed to enqueue flutter/flutter/187945 with HTTP 400: No merge queue found for branch 'main'. |
|
An existing Git SHA, To re-trigger presubmits after closing or re-opeing a PR, or pushing a HEAD commit (i.e. with |
Fixes #186343
This PR enables hot reloading of shaders when their transitive
#includedependencies are modified.Changes:
--depfiletoimpellercto generate dependency files.DevelopmentShaderCompilerusingDepfileService.