On Mon, 2010-03-15 at 07:08 -0700, michal wrote: > michal wrote on 2010-03-12 15:00: > > michal wrote on 2010-03-11 17:59: > > > >> Keith Whitwell wrote on 2010-03-11 16:16: > >> > >> > >>> On Thu, 2010-03-11 at 06:05 -0800, michal wrote: > >>> > >>> > >>> > >>>> Keith Whitwell wrote on 2010-03-11 14:21: > >>>> > >>>> > >>>> > >>>>> On Thu, 2010-03-11 at 03:16 -0800, michal wrote: > >>>>> > >>>>> > >>>>> > >>>>> > >>>>>> Hi, > >>>>>> > >>>>>> I would like to merge the branch in subject this week. This feature > >>>>>> branch allows state trackers to bind sampler views instead of textures > >>>>>> to shader stages. > >>>>>> > >>>>>> A sampler view object holds a reference to a texture and also > >>>>>> overrides > >>>>>> internal texture format (resource casting) and specifies RGBA swizzle > >>>>>> (needed for GL_EXT_texture_swizzle extension). > >>>>>> > >>>>>> > >>>>>> > >>>>>> > >>>>> Michal, > >>>>> > >>>>> I've got some issues with the way the sampler views are being generated > >>>>> and used inside the CSO module. > >>>>> > >>>>> The point of a sampler view is that it gives the driver an opportunity > >>>>> to do expensive operations required for special sampling modes (which > >>>>> may include copying surface data if hardware is deficient in some way). > >>>>> > >>>>> This approach works if a sampler view is created once, then used > >>>>> multiple times before being deleted. > >>>>> > >>>>> Unfortunately, it seems the changes to support this in the CSO module > >>>>> provide only a single-shot usage model. Sampler views are created in > >>>>> cso_set_XXX_sampler_textures, bound to the context, and then > >>>>> dereferenced/destroyed on the next bind. > >>>>> > >>>>> > >>>>> > >>>>> > >>>>> > >>>> The reason CSO code looks like this is because it was meant to be an > >>>> itermediate step towards migration to sampler view model. Fully > >>>> converting all existing state trackers is non-trivial and thus I chose > >>>> this conservative approach. State trackers that do not care about extra > >>>> features a sampler view provides will keep using this one-shot CSO > >>>> interface with the hope that creation of sampler objects is lighweight > >>>> (format matches texture format, swizzle matches native texel layout, > >>>> etc.). > >>>> > >>>> > >>>> > >>> On the surface, this hope isn't likely to be fulfilled - lots of > >>> hardware doesn't support non-zero first_level. Most cases of drivers > >>> implementing sampler views internally are to catch this issue. > >>> > >>> Of course, it seems like your branch so leaves the existing > >>> driver-specific sampler view code in place, so that there are > >>> potentially two implementations of sampler views in those drivers. > >>> > >>> I guess this means that you can get away with the current implementation > >>> for now, but it prevents drivers actually taking advantage of the fact > >>> that these entities exist in the interface -- they will continue to have > >>> to duplicate the concept internally until the state trackers and/or CSO > >>> module start caching views. > >>> > >>> > >>> > >>> > >>>> Ideally, everybody moves on and we stop using CSO for sampler > >>>> views. I prefer putting my effort into incremental migration of state > >>>> trackers rather than caching something that by definition doesn't need > >>>> to be cached. > >>>> > >>>> > >>>> > >>> The CSO module exists to manage this type of caching on behalf of state > >>> trackers. I would have thought that this was a sensible extension of > >>> the existing purpose of the CSO module. > >>> > >>> Won't all state-trackers implementing APIs which don't expose sampler > >>> views to the application require essentially the same caching logic, as > >>> is the case with regular state? Wouldn't it be least effort to do that > >>> caching once only in the CSO module? > >>> > >>> > >>> > >> OK, I see your point. I will make the necessary changes and ping you > >> when that's done. > >> > >> > >> > > Keith, > > > > I changed my mind, went ahead and implemented sampler view caching in > > mesa state tracker, rather than inside cso context. > > > > I strongly believe that doing caching on cso side would be slower and > > more complicated. A state tracker has a better understanding of the > > relationship between a texture and sampler view. In case of mesa, this > > is trivial 1-to-1 mapping. Later, when we'll need more sampler views per > > texture, we can have a per-texture cache for that, and yes, the code for > > that would be in cso. > > > > There are two other state trackers that need to be fixed: xorg and vega. > > The transition should be similar to mesa -- I can help with doing that, > > but I can't do it myself. Once that's done we can purge one-shot sampler > > view wrappers. > > > > What do you think? > > > > > Keith, > > I just finished transforming mesa and auxiliary modules to new sampler > view interfaces. The remaining bits are vega and xorg state trackers -- > I will need help with them, but they could be fixed after the merge, as > they are not broken, and just set sampler view in suboptimal fashion. > > Please review, thanks.
Michal, Did you get a chance to look at the double-refcounting (views + textures) in the cso module? Keith ------------------------------------------------------------------------------ Download Intel® Parallel Studio Eval Try the new software tools for yourself. Speed compiling, find bugs proactively, and fine-tune applications for parallel performance. See why Intel Parallel Studio got high marks during beta. http://p.sf.net/sfu/intel-sw-dev _______________________________________________ Mesa3d-dev mailing list Mesa3d-dev@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
