Last Comment Bug 782372 - Split ImageLayers.h into several files
: Split ImageLayers.h into several files
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics: Layers (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Nicolas Silva [:nical]
:
Mentors:
Depends on:
Blocks: includehell 773440
  Show dependency treegraph
 
Reported: 2012-08-13 11:48 PDT by Nicolas Silva [:nical]
Modified: 2012-08-23 09:02 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Splits ImageLayers.h into several headers (156.22 KB, patch)
2012-08-14 10:10 PDT, Nicolas Silva [:nical]
no flags Details | Diff | Splinter Review
Splits ImageLayers.h into several headers (157.89 KB, patch)
2012-08-16 07:53 PDT, Nicolas Silva [:nical]
bgirard: review+
Details | Diff | Splinter Review
Fix build (21.24 KB, patch)
2012-08-19 19:48 PDT, Matt Woodrow (:mattwoodrow)
nical.bugzilla: review+
Details | Diff | Splinter Review

Description Nicolas Silva [:nical] 2012-08-13 11:48:05 PDT
ImageLayers.h contains a lot of classes that are not all used in the same contexts. 
For example, ImageContainer is used in many places where We don't use layers.
We should separate it into several files to avoid bad dependencies.
Comment 1 Nicolas Silva [:nical] 2012-08-14 10:10:37 PDT
Created attachment 651810 [details] [diff] [review]
Splits ImageLayers.h into several headers

ImageLayers.h is splitted into the following files:
 - ImageLayers.h: contains ImageLayer
 - ImageContainer.h: contains Image, ImageContainer, and other classes derived from Image or used with Image (PlanarYCbCrImage, ImageFactory...)
 - ImageTypes.h: contains enum Image::Format, now enum ImageFormat to remove dependency between ImageLayers.h and ImageContainer.h

ImageContainer.h could be further splitted into several headers, but it would most likely be more in order to avoid a big grab-bag header rather than to have better dependencies and faster build (most users of the classes in ImageContainer.h also use ImageContainer itself fo instance, whereas most users of these classes don't use layers).

This may not be the final patch, I need to push to try once more to make sure that this does not break the build on windows (try is closed right now).
Comment 2 Nicolas Silva [:nical] 2012-08-16 07:53:22 PDT
Created attachment 652455 [details] [diff] [review]
Splits ImageLayers.h into several headers

This version is rebased from this morning and passes try
Comment 3 Benoit Girard (:BenWa) 2012-08-16 08:38:56 PDT
Comment on attachment 652455 [details] [diff] [review]
Splits ImageLayers.h into several headers

Nice! Thanks Nicolas.
Comment 5 Matt Woodrow (:mattwoodrow) 2012-08-19 19:48:08 PDT
Created attachment 653243 [details] [diff] [review]
Fix build

This patch broke my local clang builds (clang version 3.0). Attached patch fixes it :)
Comment 6 Nicolas Silva [:nical] 2012-08-19 20:00:13 PDT
Comment on attachment 653243 [details] [diff] [review]
Fix build

Review of attachment 653243 [details] [diff] [review]:
-----------------------------------------------------------------

Doh! I'll land your fix asap
Comment 8 Phil Ringnalda (:philor) 2012-08-19 21:13:54 PDT
https://hg.mozilla.org/mozilla-central/rev/54f5895b567a
Comment 9 Landry Breuil (:gaston) 2012-08-20 00:56:55 PDT
Fwiw this also affects gcc 4.2, att #653243 will fix it.
Comment 10 Ed Morley [:emorley] 2012-08-20 08:55:10 PDT
https://hg.mozilla.org/mozilla-central/rev/d948973975b7

Note You need to log in before you can comment on or make changes to this bug.