On Mon, 2010-03-15 at 07:24 -0700, michal wrote: > Keith Whitwell wrote on 2010-03-15 15:19: > > 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? > > > > > > > They will go away once all the remaining consumers of > cso_set/save/restore_sampler_textures() switch to > cso_*_fragment_sampler_views().
I think they're not needed even now - if you look at the patch I sent you, it seems like the only thing that happens to the texture pointers is they get refcounted - but never used for anything... 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
