Closed Bug 1711648 Opened 4 years ago Closed 4 years ago

Transition from rectangles to min/max representation

Categories

(Core :: Graphics: WebRender, task, P4)

Product:
Core
Component:
Graphics: WebRender
Type:
task

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: nical, Assigned: nical)

References

(Regressed 1 open bug)

Details

Attachments

(12 files, 1 obsolete file)

Bug 1711648 - Implement Peek and Poke for euclid::Box2D. r=#gfx-reviewers
48 bytes, text/x-phabricator-request
Details | Review
Bug 1711648 - Update euclid to 0.22.6. r=jrmuizel
48 bytes, text/x-phabricator-request
Details | Review
Bug 1711648 - Move all device rects to the Box2D representations. r=jrmuizel
48 bytes, text/x-phabricator-request
Details | Review
Bug 1711648 - Supress valgrind error more robustly. r=jrmuizel
48 bytes, text/x-phabricator-request
Details | Review
Bug 1711648 - Move all world rects to the endpoint representation. r=jrmuizel
48 bytes, text/x-phabricator-request
Details | Review
Bug 1711648 - Move some space conversion utilities from rect to box. r=jrmuizel
48 bytes, text/x-phabricator-request
Details | Review
Bug 1711648 - Move PictureRect to the endpoint representation. r=#gfx-reviewers
48 bytes, text/x-phabricator-request
Details | Review
Bug 1711648 - Move TileRect to the Box2D representation. r=jrmuizel
48 bytes, text/x-phabricator-request
Details | Review
Bug 1711648 - Move TileRange to the Box2D representation. r=jrmuizel
48 bytes, text/x-phabricator-request
Details | Review
Bug 1711648 - Move RasterRect to the Box2D representation. r=jrmuizel
48 bytes, text/x-phabricator-request
Details | Review
Bug 1711648 - Move LayoutRect to the Box2D representation. r=jrmuizel.
48 bytes, text/x-phabricator-request
Details | Review
Bug 1711648 - Remove redundant Box2d type. r=jrmuizel
48 bytes, text/x-phabricator-request
Details | Review

euclid::Rect uses an origin + size representation which is convenient in layout code but for most of what we do in WebRender, euclid::Box2D's endpoints presentation is both more convenient (because we reason more inter of bounds than sizes), and more efficient as we do a lot of intersection/union/contain operations. Rect internally converts to Box2D for all of these.

Keywords: leave-open
Pushed by nsilva@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1758b632b964 Update euclid to 0.22.5. r=gfx-reviewers,jrmuizel

Backed out changeset 1758b632b964 (Bug 1711648) for causing wrench bustages in reftests/transforms/prim-suite.yaml
Backout link: https://hg.mozilla.org/integration/autoland/rev/e0bab92cd0a09695dedddb23b403df62c49d5cfb
Push with failures, failure log.
(Update): Also caused valgrid failures.

Flags: needinfo?(nical.bugzilla)
Attachment #9224040 - Attachment description: Bug 1711648 - Update euclid to 0.22.5. r=#gfx-reviewers → Bug 1711648 - Update euclid to 0.22.6. r=jrmuizel
Attachment #9224331 - Attachment is obsolete: true
Pushed by nsilva@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/7d8bba559407 Update euclid to 0.22.6. r=gfx-reviewers,jrmuizel https://hg.mozilla.org/integration/autoland/rev/9cf4e612dbac Move all device rects to the Box2D representations. r=jrmuizel

Somehow I failed to submit the latest version which adjusts the reftest expectations. The valgrind failure comes from unrelated code though.

Flags: needinfo?(nical.bugzilla)
Pushed by nsilva@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/71807d4e6302 Update euclid to 0.22.6. r=gfx-reviewers,jrmuizel https://hg.mozilla.org/integration/autoland/rev/7f1ae1fdd305 Move all device rects to the Box2D representations. r=jrmuizel
Pushed by abutkovits@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c6c83dc97cbb Update euclid to 0.22.6. r=jrmuizel https://hg.mozilla.org/integration/autoland/rev/3d51bfaa0d8d Move all device rects to the Box2D representations. r=jrmuizel

Relanded, sorry for the wrongfull backout.

Flags: needinfo?(nical.bugzilla)

Backed out for causing valgrind bustages

This has started failing again once the changesets have been relanded: https://treeherder.mozilla.org/jobs?repo=autoland&fromchange=1c110c9d06c3f38dbee3d91d1ac30bad4f721cf2&searchStr=linux%2Cx64%2Copt%2Cvalgrind-linux64-valgrind%2Fopt%2Cv&tochange=32e3e98ca8474ded656e7d6eb877fa1919e5355b&selectedTaskRun=U5pJynXFRO-dRoKSevkH8A.0

[task 2021-06-02T06:44:17.512Z] 06:44:17     INFO -   1:26.66 TEST-UNEXPECTED-FAIL | valgrind-test | Conditional jump or move depends on uninitialised value(s) at style::properties::shorthands::font::parse_into / style::properties::PropertyDeclaration::parse_into / style::properties::declaration_block::parse_property_declaration_list / ?!?
[task 2021-06-02T06:44:17.512Z] 06:44:17     INFO -   1:26.66 ==12420== Conditional jump or move depends on uninitialised value(s)
[task 2021-06-02T06:44:17.512Z] 06:44:17     INFO -   1:26.66 ==12420==    at 0x15E6028F: style::properties::shorthands::font::parse_into+3663 (mod.rs:187)
[task 2021-06-02T06:44:17.512Z] 06:44:17     INFO -   1:26.66 ==12420==    by 0x15E47FE8: style::properties::PropertyDeclaration::parse_into+1336 (properties.rs:43921)
[task 2021-06-02T06:44:17.512Z] 06:44:17     INFO -   1:26.66 ==12420==    by 0x15F18C4C: style::properties::declaration_block::parse_property_declaration_list+3132 (declaration_block.rs:1387)
[task 2021-06-02T06:44:17.513Z] 06:44:17     INFO -   1:26.66 ==12420==    by 0x15F21B13: <style::stylesheets::rule_parser::NestedRuleParser as cssparser::rules_and_declarations::QualifiedRuleParser>::parse_block+131 (rule_parser.rs:649)
[task 2021-06-02T06:44:17.513Z] 06:44:17     INFO -   1:26.66 ==12420==    by 0x15F0D39B: cssparser::rules_and_declarations::parse_qualified_rule+1643 (rule_parser.rs:331)
[task 2021-06-02T06:44:17.513Z] 06:44:17     INFO -   1:26.66 ==12420==    by 0x15F2CD0C: style::stylesheets::stylesheet::StylesheetContents::from_str+2828 (rules_and_declarations.rs:400)
[task 2021-06-02T06:44:17.513Z] 06:44:17     INFO -   1:26.66 ==12420==    by 0x158A6627: Servo_StyleSheet_FromUTF8Bytes+455 (glue.rs:1565)
[task 2021-06-02T06:44:17.513Z] 06:44:17     INFO -   1:26.66 ==12420==    by 0x139F74B9: mozilla::StyleSheet::ParseSheetSync(mozilla::css::Loader*, nsTSubstring<char> const&, mozilla::css::SheetLoadData*, unsigned int, mozilla::css::LoaderReusableStyleSheets*)+217 (checkouts/gecko/layout/style/StyleSheet.cpp:1248)
[task 2021-06-02T06:44:17.514Z] 06:44:17     INFO -   1:26.66 ==12420==    by 0x139E3C5B: mozilla::css::Loader::ParseSheet(nsTSubstring<char> const&, mozilla::css::SheetLoadData&, mozilla::css::Loader::AllowAsyncParse)+555 (checkouts/gecko/layout/style/Loader.cpp:1524)
[task 2021-06-02T06:44:17.514Z] 06:44:17     INFO -   1:26.66 ==12420==    by 0x139F2511: mozilla::css::StreamLoader::OnStopRequest(nsIRequest*, nsresult)+561 (checkouts/gecko/layout/style/StreamLoader.cpp:143)
[task 2021-06-02T06:44:17.514Z] 06:44:17     INFO -   1:26.66 ==12420==    by 0x11FE1844: nsSyncLoadService::PushSyncStreamToListener(already_AddRefed<nsIInputStream>, nsIStreamListener*, nsIChannel*)+372 (checkouts/gecko/dom/base/nsSyncLoadService.cpp:360)
[task 2021-06-02T06:44:17.514Z] 06:44:17     INFO -   1:26.66 ==12420==    by 0x139E2E66: mozilla::css::Loader::LoadSheet(mozilla::css::SheetLoadData&, mozilla::css::Loader::SheetState, mozilla::css::Loader::PendingLoad)+1990 (checkouts/gecko/layout/style/Loader.cpp:1291)
[task 2021-06-02T06:44:17.515Z] 06:44:17     INFO -   1:26.66 ==12420==    by 0x139E5D44: mozilla::css::Loader::LoadChildSheet(mozilla::StyleSheet&, mozilla::css::SheetLoadData*, nsIURI*, mozilla::dom::MediaList*, mozilla::css::LoaderReusableStyleSheets*)+1204 (checkouts/gecko/layout/style/Loader.cpp:2013)
[task 2021-06-02T06:44:17.515Z] 06:44:17     INFO -   1:26.66 ==12420==    by 0x139D46AC: LoadImportSheet(mozilla::css::Loader*, mozilla::StyleSheet*, mozilla::css::SheetLoadData*, mozilla::css::LoaderReusableStyleSheets*, mozilla::StyleCssUrl const&, already_AddRefed<RawServoMediaList>)+188 (checkouts/gecko/layout/style/GeckoBindings.cpp:1512)
[task 2021-06-02T06:44:17.515Z] 06:44:17     INFO -   1:26.66 ==12420==    by 0x158A674E: <geckoservo::stylesheet_loader::StylesheetLoader as style::stylesheets::loader::StylesheetLoader>::request_stylesheet+62 (stylesheet_loader.rs:62)
[task 2021-06-02T06:44:17.515Z] 06:44:17     INFO -   1:26.66 ==12420==    by 0x15F0E2AA: cssparser::rules_and_declarations::parse_at_rule+3018 (rule_parser.rs:274)
[task 2021-06-02T06:44:17.515Z] 06:44:17     INFO -   1:26.66 ==12420==    by 0x15F2C9DC: style::stylesheets::stylesheet::StylesheetContents::from_str+2012 (rules_and_declarations.rs:391)
[task 2021-06-02T06:44:17.516Z] 06:44:17     INFO -   1:26.66 ==12420==    by 0x158A6627: Servo_StyleSheet_FromUTF8Bytes+455 (glue.rs:1565)
[task 2021-06-02T06:44:17.516Z] 06:44:17     INFO -   1:26.66 ==12420==    by 0x139F74B9: mozilla::StyleSheet::ParseSheetSync(mozilla::css::Loader*, nsTSubstring<char> const&, mozilla::css::SheetLoadData*, unsigned int, mozilla::css::LoaderReusableStyleSheets*)+217 (checkouts/gecko/layout/style/StyleSheet.cpp:1248)
[task 2021-06-02T06:44:17.516Z] 06:44:17     INFO -   1:26.66 ==12420==    by 0x139E3C5B: mozilla::css::Loader::ParseSheet(nsTSubstring<char> const&, mozilla::css::SheetLoadData&, mozilla::css::Loader::AllowAsyncParse)+555 (checkouts/gecko/layout/style/Loader.cpp:1524)
[task 2021-06-02T06:44:17.516Z] 06:44:17     INFO -   1:26.66 ==12420==    by 0x139F2511: mozilla::css::StreamLoader::OnStopRequest(nsIRequest*, nsresult)+561 (checkouts/gecko/layout/style/StreamLoader.cpp:143)
[task 2021-06-02T06:44:17.517Z] 06:44:17     INFO -   1:26.66 ==12420==    by 0x11FE1844: nsSyncLoadService::PushSyncStreamToListener(already_AddRefed<nsIInputStream>, nsIStreamListener*, nsIChannel*)+372 (checkouts/gecko/dom/base/nsSyncLoadService.cpp:360)
[task 2021-06-02T06:44:17.517Z] 06:44:17     INFO -   1:26.66 ==12420==    by 0x139E2E66: mozilla::css::Loader::LoadSheet(mozilla::css::SheetLoadData&, mozilla::css::Loader::SheetState, mozilla::css::Loader::PendingLoad)+1990 (checkouts/gecko/layout/style/Loader.cpp:1291)
[task 2021-06-02T06:44:17.517Z] 06:44:17     INFO -   1:26.66 ==12420==    by 0x139E6466: mozilla::css::Loader::InternalLoadNonDocumentSheet(nsIURI*, mozilla::css::StylePreloadKind, mozilla::css::SheetParsingMode, mozilla::css::Loader::UseSystemPrincipal, mozilla::Encoding const*, nsIReferrerInfo*, nsICSSLoaderObserver*, mozilla::CORSMode, nsTSubstring<char16_t> const&)+678 (checkouts/gecko/layout/style/Loader.cpp:2106)
[task 2021-06-02T06:44:17.517Z] 06:44:17     INFO -   1:26.66 ==12420==    by 0x139E614B: mozilla::css::Loader::LoadSheetSync(nsIURI*, mozilla::css::SheetParsingMode, mozilla::css::Loader::UseSystemPrincipal)+219 (checkouts/gecko/layout/style/Loader.cpp:2027)
[task 2021-06-02T06:44:17.518Z] 06:44:17     INFO -   1:26.66 ==12420==    by 0x139D7AF9: mozilla::GlobalStyleSheetCache::LoadSheet(nsIURI*, mozilla::css::SheetParsingMode, mozilla::css::FailureAction)+89 (checkouts/gecko/layout/style/GlobalStyleSheetCache.cpp:545)
[task 2021-06-02T06:44:17.518Z] 06:44:17     INFO -   1:26.66 ==12420==    by 0x139D6CE5: LoadSheetURL (checkouts/gecko/layout/style/GlobalStyleSheetCache.cpp:497)
[task 2021-06-02T06:44:17.518Z] 06:44:17     INFO -   1:26.66 ==12420==    by 0x139D6CE5: mozilla::GlobalStyleSheetCache::XULSheet()+85 (dist/include/mozilla/UserAgentStyleSheetList.h:36)
[task 2021-06-02T06:44:17.518Z] 06:44:17     INFO -   1:26.66 ==12420==    by 0x139D77AD: mozilla::GlobalStyleSheetCache::GlobalStyleSheetCache()+189 (checkouts/gecko/layout/style/GlobalStyleSheetCache.cpp:239)
[task 2021-06-02T06:44:17.518Z] 06:44:17     INFO -   1:26.66 ==12420==    by 0x139D8717: mozilla::GlobalStyleSheetCache::Singleton()+39 (checkouts/gecko/layout/style/GlobalStyleSheetCache.cpp:447)
[task 2021-06-02T06:44:17.519Z] 06:44:17     INFO -   1:26.66 ==12420==    by 0x1200F428: mozilla::dom::Document::FillStyleSetUserAndUASheets()+24 (checkouts/gecko/dom/base/Document.cpp:3001)
[task 2021-06-02T06:44:17.519Z] 06:44:17     INFO -   1:26.66 ==12420==    by 0x1201BD6C: FillStyleSet (checkouts/gecko/dom/base/Document.cpp:3065)
[task 2021-06-02T06:44:17.519Z] 06:44:17     INFO -   1:26.66 ==12420==    by 0x1201BD6C: mozilla::dom::Document::CreatePresShell(nsPresContext*, nsViewManager*)+124 (checkouts/gecko/dom/base/Document.cpp:6624)
[task 2021-06-02T06:44:17.519Z] 06:44:17     INFO -   1:26.66 ==12420==    by 0x13A93B76: nsDocumentViewer::InitPresentationStuff(bool)+102 (checkouts/gecko/layout/base/nsDocumentViewer.cpp:705)
[task 2021-06-02T06:44:17.519Z] 06:44:17     INFO -   1:26.66 ==12420==    by 0x13A93A89: nsDocumentViewer::InitInternal(nsIWidget*, nsISupports*, mozilla::dom::WindowGlobalChild*, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, bool, bool, bool)+1193 (checkouts/gecko/layout/base/nsDocumentViewer.cpp:910)
[task 2021-06-02T06:44:17.520Z] 06:44:17     INFO -   1:26.66 ==12420==    by 0x13A935D7: nsDocumentViewer::Init(nsIWidget*, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, mozilla::dom::WindowGlobalChild*)+23 (checkouts/gecko/layout/base/nsDocumentViewer.cpp:681)
[task 2021-06-02T06:44:17.520Z] 06:44:17     INFO -   1:26.66 ==12420==    by 0x148133AA: nsDocShell::SetupNewViewer(nsIContentViewer*, mozilla::dom::WindowGlobalChild*)+714 (checkouts/gecko/docshell/base/nsDocShell.cpp:8265)
[task 2021-06-02T06:44:17.520Z] 06:44:17     INFO -   1:26.66 ==12420==    by 0x14812EF2: nsDocShell::Embed(nsIContentViewer*, mozilla::dom::WindowGlobalChild*, bool, bool)+50 (checkouts/gecko/docshell/base/nsDocShell.cpp:5779)
[task 2021-06-02T06:44:17.520Z] 06:44:17     INFO -   1:26.66 ==12420==  Uninitialised value was created by a stack allocation
[task 2021-06-02T06:44:17.520Z] 06:44:17     INFO -   1:26.66 ==12420==    at 0x15E5F44D: style::properties::shorthands::font::parse_into+13 (font.rs:301)
[task 2021-06-02T06:44:17.521Z] 06:44:17     INFO -   1:26.66 ==12420==
Flags: needinfo?(nical.bugzilla)

The problem here is that there is a valgrind supression for this error but it is set on the exact signature of the generic function and that signature contains a hash generated by rustc that changes when some crate dependencies changes.

Flags: needinfo?(nical.bugzilla)

There is a valgrind suppression on a generic signature containing the hash of rust types. The hash can change when crate dependencies are updated.

Pushed by nsilva@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/845ee2c0984b Supress valgrind error more robustly. r=jrmuizel https://hg.mozilla.org/integration/autoland/rev/f30a47e089f5 Update euclid to 0.22.6. r=gfx-reviewers,jrmuizel https://hg.mozilla.org/integration/autoland/rev/1bd839204389 Move all device rects to the Box2D representations. r=jrmuizel
Regressions: 1714926
Pushed by nsilva@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ee22f8569e3c Move all world rects to the endpoint representation. r=jrmuizel
Pushed by nsilva@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/bdbc65799b8a Move some space conversion utilities from rect to box. r=jrmuizel https://hg.mozilla.org/integration/autoland/rev/99ce7c7e458d Move PictureRect to the endpoint representation. r=gfx-reviewers,jrmuizel https://hg.mozilla.org/integration/autoland/rev/602a2bcc5e29 Move TileRect to the Box2D representation. r=jrmuizel https://hg.mozilla.org/integration/autoland/rev/823b75fc8c3c Move TileRange to the Box2D representation. r=jrmuizel https://hg.mozilla.org/integration/autoland/rev/d07c38d536c5 Move RasterRect to the Box2D representation. r=jrmuizel https://hg.mozilla.org/integration/autoland/rev/a803c960b909 Move LayoutRect to the Box2D representation. r=jrmuizel.

Backed out 6 changesets (Bug 1711648) for causing build bustages.
Backout link
Push with failures
Failure Log

Pushed by nsilva@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/367235e897e3 Move some space conversion utilities from rect to box. r=jrmuizel https://hg.mozilla.org/integration/autoland/rev/1d4c0b685f0e Move PictureRect to the endpoint representation. r=gfx-reviewers,jrmuizel https://hg.mozilla.org/integration/autoland/rev/12da58f4ac4f Move TileRect to the Box2D representation. r=jrmuizel https://hg.mozilla.org/integration/autoland/rev/b27d8421ebc5 Move TileRange to the Box2D representation. r=jrmuizel https://hg.mozilla.org/integration/autoland/rev/15d8e0d681ef Move RasterRect to the Box2D representation. r=jrmuizel https://hg.mozilla.org/integration/autoland/rev/0384828b36cc Move LayoutRect to the Box2D representation. r=jrmuizel.
Backout by malexandru@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/86f991c484b5 Backed out 6 changesets for causing crashes in FrameBuilder.

Backed out 6 changesets (Bug 1711648) for causing crashes in FrameBuilder.
Backout link: https://hg.mozilla.org/integration/autoland/rev/86f991c484b5da6b66c55869634b8eb830956642
Push with failures, failure logs: wpt, mochitest plain.

Flags: needinfo?(nical.bugzilla)
Pushed by nsilva@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/4236adac136f Move some space conversion utilities from rect to box. r=jrmuizel https://hg.mozilla.org/integration/autoland/rev/348357d42d90 Move PictureRect to the endpoint representation. r=gfx-reviewers,jrmuizel https://hg.mozilla.org/integration/autoland/rev/9cab55e23f41 Move TileRect to the Box2D representation. r=jrmuizel https://hg.mozilla.org/integration/autoland/rev/a8d1613dc2a0 Move TileRange to the Box2D representation. r=jrmuizel https://hg.mozilla.org/integration/autoland/rev/8c8df8e29ab2 Move RasterRect to the Box2D representation. r=jrmuizel https://hg.mozilla.org/integration/autoland/rev/1e0d4123efb1 Move LayoutRect to the Box2D representation. r=jrmuizel.
Flags: needinfo?(nical.bugzilla)
Regressions: 1717655
Attachment #9231547 - Attachment description: Bug 1711648 - Remove redundant Box2d type. r=jrmuizel. → Bug 1711648 - Remove redundant Box2d type. r=jrmuizel
Pushed by nsilva@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b63591c3949e Remove redundant Box2d type. r=jrmuizel
Status: NEW → RESOLVED
Closed: 4 years ago
Keywords: leave-open
Resolution: --- → FIXED
Regressions: 1725179
Regressions: 1736569
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: