Merge amethyst#1859 amethyst#1866

1859: Improve naming for UI prefab structs and export UiButtonData r=azriel91,barskern a=happenslol ## Description The naming for the structs that UI prefab data is deserialized from was very confusing. They were previously called `UiTransformBuilder`, `UiButtonBuilder`, and so on, but were mainly used for deserializing toml files into prefabs. Additionally, if you wanted to return a button from a custom widget, you would need to return `UiButtonBuilder`, which was never exported. This PR changes the naming to better reflect what the structs are actually for, and exports all of them. ## Modifications - All `-Builder` structs in `amethyst_ui/prefab.rs` are now called `-Data`. ## PR Checklist By placing an x in the boxes I certify that I have: - [x] Updated the content of the book if this PR would make the book outdated. - [x] Added a changelog entry if this will impact users, or modified more than 5 lines of Rust that wasn't a doc comment. - [ ] Added unit tests for new code added in this PR. - [x] Acknowledged that by making this pull request I release this code under an MIT/Apache 2.0 dual licensing scheme. If this modified or created any rs files: - [x] Ran `cargo +stable fmt --all` - [x] Ran `cargo clippy --all --features "empty"` - [x] Ran `cargo test --all --features "empty"` 1866: Expose ShadeStageFlags for EnvironmentSub r=Frizi,azriel91 a=valkum ## Description Allows to set the ShaderStageFlags when using the EnvironmentSub. Privously this was fixed to the vertex shader for the first uniform buffer and the fragment shader for the second uniform buffer. This does not check if the flags have a stage in common. ## Modifications - `EnvironmentSub::new` takes a new argument flags of type [gfx_hal::pso::ShaderStageFlags; 2] One flag for each of the two uniform buffers. Similar to `DynamicUniform::new` ## PR Checklist By placing an x in the boxes I certify that I have: - [ ] Updated the content of the book if this PR would make the book outdated. - [ ] Added a changelog entry if this will impact users, or modified more than 5 lines of Rust that wasn't a doc comment. - [ ] Added unit tests for new code added in this PR. - [x] Acknowledged that by making this pull request I release this code under an MIT/Apache 2.0 dual licensing scheme. If this modified or created any rs files: - [x] Ran `cargo +stable fmt --all` - [ ] Ran `cargo clippy --all --features "empty"` - [x] Ran `cargo test --all --features "empty"` Co-authored-by: Hilmar Wiegand <[email protected]> Co-authored-by: Azriel Hoh <[email protected]> Co-authored-by: Rudi Floren <[email protected]>
leniseev · Aug 9, 2019 · 2bb0c6e · 2bb0c6e
3 parents 61dc488 + 26ef6cb + 37af163
commit 2bb0c6e
Show file tree

Hide file tree

Showing 10 changed files with 65 additions and 40 deletions.
diff --git a/amethyst_rendy/src/macros.rs b/amethyst_rendy/src/macros.rs
@@ -1,12 +1,12 @@
 macro_rules! set_layout {
-    ($factory:expr, $([$times:expr] $ty:ident $flags:ident),*) => {
+    ($factory:expr, $([$times:expr] $ty:ident $flags:expr),*) => {
         $factory.create_descriptor_set_layout(
             crate::util::set_layout_bindings(
                 std::iter::empty()
                     $(.chain(std::iter::once((
                         $times as u32,
                         rendy::hal::pso::DescriptorType::$ty,
-                        rendy::hal::pso::ShaderStageFlags::$flags
+                        $flags
                     ))))*
             )
         )?.into()

diff --git a/amethyst_rendy/src/pass/base_3d.rs b/amethyst_rendy/src/pass/base_3d.rs
@@ -112,7 +112,13 @@ impl<B: Backend, T: Base3DPassDef> RenderGroupDesc<B, World> for DrawBase3DDesc<
     ) -> Result<Box<dyn RenderGroup<B, World>>, failure::Error> {
         profile_scope_impl!("build");
 
-        let env = EnvironmentSub::new(factory)?;
+        let env = EnvironmentSub::new(
+            factory,
+            [
+                hal::pso::ShaderStageFlags::VERTEX,
+                hal::pso::ShaderStageFlags::FRAGMENT,
+            ],
+        )?;
         let materials = MaterialSub::new(factory)?;
         let skinning = SkinningSub::new(factory)?;
 
@@ -426,7 +432,14 @@ impl<B: Backend, T: Base3DPassDef> RenderGroupDesc<B, World> for DrawBase3DTrans
         _buffers: Vec<NodeBuffer>,
         _images: Vec<NodeImage>,
     ) -> Result<Box<dyn RenderGroup<B, World>>, failure::Error> {
-        let env = EnvironmentSub::new(factory)?;
+        let env = EnvironmentSub::new(
+            factory,
+            [
+                hal::pso::ShaderStageFlags::VERTEX,
+                hal::pso::ShaderStageFlags::FRAGMENT,
+            ],
+        )?;
+
         let materials = MaterialSub::new(factory)?;
         let skinning = SkinningSub::new(factory)?;
 

diff --git a/amethyst_rendy/src/submodules/environment.rs b/amethyst_rendy/src/submodules/environment.rs
@@ -48,9 +48,13 @@ struct PerImageEnvironmentSub<B: Backend> {
 
 impl<B: Backend> EnvironmentSub<B> {
     /// Create and allocate a new `EnvironmentSub` with the provided rendy `Factory`
-    pub fn new(factory: &Factory<B>) -> Result<Self, failure::Error> {
+    /// Allocate to the supplied shader.
+    pub fn new(
+        factory: &Factory<B>,
+        flags: [hal::pso::ShaderStageFlags; 2],
+    ) -> Result<Self, failure::Error> {
         Ok(Self {
-            layout: set_layout! {factory, [1] UniformBuffer VERTEX, [4] UniformBuffer FRAGMENT},
+            layout: set_layout! {factory, [1] UniformBuffer flags[0], [4] UniformBuffer flags[1]},
             per_image: Vec::new(),
         })
     }

diff --git a/amethyst_rendy/src/submodules/material.rs b/amethyst_rendy/src/submodules/material.rs
@@ -152,7 +152,11 @@ impl<B: Backend, T: for<'a> StaticTextureSet<'a>> MaterialSub<B, T> {
     /// Create a new `MaterialSub` using the provided rendy `Factory`
     pub fn new(factory: &Factory<B>) -> Result<Self, failure::Error> {
         Ok(Self {
-            layout: set_layout! {factory, [1] UniformBuffer FRAGMENT, [T::len()] CombinedImageSampler FRAGMENT},
+            layout: set_layout! {
+                factory,
+                [1] UniformBuffer hal::pso::ShaderStageFlags::FRAGMENT,
+                [T::len()] CombinedImageSampler hal::pso::ShaderStageFlags::FRAGMENT
+            },
             lookup: util::LookupBuilder::new(),
             allocator: SlotAllocator::new(1024),
             buffers: vec![Self::create_buffer(factory)?],

diff --git a/amethyst_rendy/src/submodules/skinning.rs b/amethyst_rendy/src/submodules/skinning.rs
@@ -35,7 +35,7 @@ impl<B: Backend> SkinningSub<B> {
     /// Create a new `SkinningSub`, allocating using the provided `Factory`
     pub fn new(factory: &Factory<B>) -> Result<Self, failure::Error> {
         Ok(Self {
-            layout: set_layout! {factory, [1] StorageBuffer VERTEX},
+            layout: set_layout! {factory, [1] StorageBuffer hal::pso::ShaderStageFlags::VERTEX},
             skin_offset_map: Default::default(),
             staging: Vec::new(),
             per_image: Vec::new(),

diff --git a/amethyst_rendy/src/submodules/texture.rs b/amethyst_rendy/src/submodules/texture.rs
@@ -46,7 +46,7 @@ impl<B: Backend> TextureSub<B> {
     /// Create a new Texture for submission, allocated using the provided `Factory`
     pub fn new(factory: &Factory<B>) -> Result<Self, failure::Error> {
         Ok(Self {
-            layout: set_layout! {factory, [1] CombinedImageSampler FRAGMENT},
+            layout: set_layout! {factory, [1] CombinedImageSampler hal::pso::ShaderStageFlags::FRAGMENT},
             lookup: util::LookupBuilder::new(),
             textures: Vec::with_capacity(1024),
             generation: 0,

diff --git a/amethyst_ui/src/lib.rs b/amethyst_ui/src/lib.rs
@@ -30,8 +30,8 @@ pub use self::{
     layout::{Anchor, ScaleMode, Stretch, UiTransformSystem, UiTransformSystemDesc},
     pass::{DrawUi, DrawUiDesc, RenderUi},
     prefab::{
-        NoCustomUi, ToNativeWidget, UiCreator, UiFormat, UiImagePrefab, UiLoader, UiLoaderSystem,
-        UiLoaderSystemDesc, UiPrefab, UiTextBuilder, UiTransformBuilder, UiWidget,
+        NoCustomUi, ToNativeWidget, UiButtonData, UiCreator, UiFormat, UiImagePrefab, UiLoader,
+        UiLoaderSystem, UiLoaderSystemDesc, UiPrefab, UiTextData, UiTransformData, UiWidget,
     },
     resize::{ResizeSystem, ResizeSystemDesc, UiResize},
     selection::{

diff --git a/amethyst_ui/src/prefab.rs b/amethyst_ui/src/prefab.rs
@@ -33,7 +33,7 @@ use crate::{
 #[derive(Debug, Clone, Deserialize, Serialize, Derivative)]
 #[serde(default)]
 #[derivative(Default)]
-pub struct UiTransformBuilder<G> {
+pub struct UiTransformData<G> {
     /// An identifier. Serves no purpose other than to help you distinguish between UI elements.
     pub id: String,
     /// X coordinate
@@ -79,7 +79,7 @@ pub struct UiTransformBuilder<G> {
     _phantom: PhantomData<G>,
 }
 
-impl<G> UiTransformBuilder<G> {
+impl<G> UiTransformData<G> {
     /// Set id
     pub fn with_id<S>(mut self, id: S) -> Self
     where
@@ -135,7 +135,7 @@ impl<G> UiTransformBuilder<G> {
     }
 }
 
-impl<'a, G> PrefabData<'a> for UiTransformBuilder<G>
+impl<'a, G> PrefabData<'a> for UiTransformData<G>
 where
     G: Send + Sync + 'static,
 {
@@ -197,7 +197,7 @@ where
 /// - `F`: `Format` used for loading `FontAsset`
 /// - `G`: Selection Group Type
 #[derive(Deserialize, Serialize, Clone)]
-pub struct UiTextBuilder {
+pub struct UiTextData {
     /// Text to display
     pub text: String,
     /// Font size
@@ -217,7 +217,7 @@ pub struct UiTextBuilder {
     #[serde(default)]
     pub editable: Option<TextEditingPrefab>,
 }
-impl Debug for UiTextBuilder {
+impl Debug for UiTextData {
     fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
         let font = match self.font.as_ref() {
             Some(asset_prefab) => match asset_prefab {
@@ -227,7 +227,7 @@ impl Debug for UiTextBuilder {
             _ => "<Font>".to_string(),
         };
 
-        f.debug_struct("UiTextBuilder")
+        f.debug_struct("UiTextData")
             .field("text", &self.text)
             .field("font_size", &self.font_size)
             .field("font", &font)
@@ -265,7 +265,7 @@ impl Default for TextEditingPrefab {
     }
 }
 
-impl<'a> PrefabData<'a> for UiTextBuilder {
+impl<'a> PrefabData<'a> for UiTextData {
     type SystemData = (
         WriteStorage<'a, UiText>,
         WriteStorage<'a, TextEditing>,
@@ -418,7 +418,7 @@ impl<'a> PrefabData<'a> for UiImageLoadPrefab {
 ///
 /// - `W`: Type used for Widget IDs
 #[derive(Deserialize, Serialize, Clone)]
-pub struct UiButtonBuilder<W: WidgetId = u32> {
+pub struct UiButtonData<W: WidgetId = u32> {
     /// Id for the widget
     pub id: Option<W>,
     /// Text to display
@@ -450,7 +450,7 @@ pub struct UiButtonBuilder<W: WidgetId = u32> {
     pub release_sound: Option<AssetPrefab<Audio>>,
 }
 
-impl<W: WidgetId + Debug> Debug for UiButtonBuilder<W> {
+impl<W: WidgetId + Debug> Debug for UiButtonData<W> {
     fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
         let font = match self.font.as_ref() {
             Some(asset_prefab) => match asset_prefab {
@@ -460,7 +460,7 @@ impl<W: WidgetId + Debug> Debug for UiButtonBuilder<W> {
             _ => "<Font>".to_string(),
         };
 
-        f.debug_struct("UiTextBuilder")
+        f.debug_struct("UiTextData")
             .field("id", &self.id)
             .field("text", &self.text)
             .field("font_size", &self.font_size)
@@ -477,7 +477,7 @@ impl<W: WidgetId + Debug> Debug for UiButtonBuilder<W> {
     }
 }
 
-impl<'a, W> PrefabData<'a> for UiButtonBuilder<W>
+impl<'a, W> PrefabData<'a> for UiButtonData<W>
 where
     W: WidgetId,
 {
@@ -645,7 +645,7 @@ where
     /// Container widget
     Container {
         /// Spatial information for the container
-        transform: UiTransformBuilder<G>,
+        transform: UiTransformData<G>,
         /// Background image
         #[serde(default = "default_container_image")]
         background: Option<UiImagePrefab>,
@@ -655,23 +655,23 @@ where
     /// Image widget
     Image {
         /// Spatial information
-        transform: UiTransformBuilder<G>,
+        transform: UiTransformData<G>,
         /// Image
         image: UiImagePrefab,
     },
     /// Text widget
     Label {
         /// Spatial information
-        transform: UiTransformBuilder<G>,
+        transform: UiTransformData<G>,
         /// Text
-        text: UiTextBuilder,
+        text: UiTextData,
     },
     /// Button widget
     Button {
         /// Spatial information
-        transform: UiTransformBuilder<G>,
+        transform: UiTransformData<G>,
         /// Button
-        button: UiButtonBuilder<W>,
+        button: UiButtonData<W>,
     },
     /// Custom UI widget
     Custom(Box<C>),
@@ -682,8 +682,8 @@ where
     C: ToNativeWidget<W>,
     W: WidgetId,
 {
-    /// Convenience function to access widgets `UiTransformBuilder`
-    pub fn transform(&self) -> Option<&UiTransformBuilder<G>> {
+    /// Convenience function to access widgets `UiTransformData`
+    pub fn transform(&self) -> Option<&UiTransformData<G>> {
         match self {
             UiWidget::Container { ref transform, .. } => Some(transform),
             UiWidget::Image { ref transform, .. } => Some(transform),
@@ -693,8 +693,8 @@ where
         }
     }
 
-    /// Convenience function to access widgets `UiTransformBuilder`
-    pub fn transform_mut(&mut self) -> Option<&mut UiTransformBuilder<G>> {
+    /// Convenience function to access widgets `UiTransformData`
+    pub fn transform_mut(&mut self) -> Option<&mut UiTransformData<G>> {
         match self {
             UiWidget::Container {
                 ref mut transform, ..
@@ -771,10 +771,10 @@ fn default_container_image() -> Option<UiImagePrefab> {
 }
 
 type UiPrefabData<D = <NoCustomUi as ToNativeWidget>::PrefabData, W = u32, G = ()> = (
-    Option<UiTransformBuilder<G>>,
+    Option<UiTransformData<G>>,
     Option<UiImagePrefab>,
-    Option<UiTextBuilder>,
-    Option<UiButtonBuilder<W>>,
+    Option<UiTextData>,
+    Option<UiButtonData<W>>,
     D,
 );
 
@@ -872,7 +872,7 @@ fn walk_ui_tree<C, W>(
             mut button,
         } => {
             let id = transform.id.clone();
-            let text = UiTextBuilder {
+            let text = UiTextData {
                 color: button.normal_text_color,
                 editable: None,
                 font: button.font.clone(),
@@ -1023,9 +1023,9 @@ pub type UiLoaderSystemDesc<CD, W> = PrefabLoaderSystemDesc<UiPrefabData<CD, W>>
 /// - `W`: Type used for Widget IDs
 pub type UiLoaderSystem<CD, W> = PrefabLoaderSystem<UiPrefabData<CD, W>>;
 
-fn button_text_transform<G>(mut id: String) -> UiTransformBuilder<G> {
+fn button_text_transform<G>(mut id: String) -> UiTransformData<G> {
     id.push_str("_btn_txt");
-    UiTransformBuilder::default()
+    UiTransformData::default()
         .with_id(id)
         .with_position(0., 0., 1.)
         .with_anchor(Anchor::Middle)

diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md
@@ -17,13 +17,17 @@ The format is based on [Keep a Changelog][kc], and this project adheres to
 ### Added
 
 * `SystemDesc` proc macro derive to simplify defining `SystemDesc`s. ([#1780])
+* `UiButtonData` is now exported from `amethyst_ui` and can be used for custom widgets. ([#1859])
 * Add an audio subchapter to the pong chapter. ([#1842])
 
 ### Changed
 
+* All `-Builder` structs in amethyst_ui/prefab.rs are now called `-Data`. ([#1859])
+
 ### Fixed
 
 [#1780]: https://github.com/amethyst/amethyst/pull/1780
+[#1859]: https://github.com/amethyst/amethyst/pull/1859
 [#1842]: https://github.com/amethyst/amethyst/pull/1842
 
 ## [0.12.0] - 2019-07-30

diff --git a/examples/custom_ui/main.rs b/examples/custom_ui/main.rs
@@ -12,7 +12,7 @@ use amethyst::{
         types::DefaultBackend,
         RenderingBundle,
     },
-    ui::{RenderUi, ToNativeWidget, UiBundle, UiCreator, UiTransformBuilder, UiWidget},
+    ui::{RenderUi, ToNativeWidget, UiBundle, UiCreator, UiTransformData, UiWidget},
     utils::{application_root_dir, scene::BasicScenePrefab},
 };
 
@@ -51,7 +51,7 @@ impl ToNativeWidget for CustomUi {
                     .map(|widget| {
                         let widget = UiWidget::Container {
                             background: None,
-                            transform: UiTransformBuilder::default()
+                            transform: UiTransformData::default()
                                 .with_position(pos.0, pos.1, pos.2),
                             children: vec![widget],
                         };